<div dir="ltr"><div style>All is true. But average UUID.fromString execution is 1675us, and HashMap.put is 78us - so the benefit is clear when we're talking on >100K executions for 10minutes...</div></div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Sun, Jun 30, 2013 at 12:44 PM, Michael Pasternak <span dir="ltr"><<a href="mailto:mpastern@redhat.com" target="_blank">mpastern@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 06/30/2013 12:20 PM, Liran Zelkha wrote:<br>
> I checked such a solution using JProfiler. Creating the GUID object takes much more CPU cycles that checking the string in the Map.<br>
<br>
</div>of course it is, but what is the size of map that you checked?, check on worst scenario, i.e map<br>
is full of all possible guids,<br>
<br>
also problem a bit different,java map has a load factor (which is usually 0.75),<br>
when ratio increases beyond the load factor, occurs proses called re-hash so that the hash<br>
table will double amount of buckets. what can produce a cpu spikes (though it should not happen too often),<br>
to avoid this the initial capacity should be greater than the maximum number of entries / the<br>
load factor, and this is a huge map....<br>
<br>
so basically this is a tradeoff between time and space costs against the new guid generation.<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> On Jun 30, 2013, at 12:06 PM, Michael Pasternak wrote:<br>
><br>
>> On 06/30/2013 11:37 AM, Liran Zelkha wrote:<br>
>>> Great news.<br>
>>> Allon - please note that GUID is being recreated from String by both DB calls and by data received from VDSM. It is VERY useful to cache Guid String-->Guid, and not just Empty GUID.<br>
>>><br>
>>> However, as the Guid class runs in GWT as well, you can't use Infinispan and you're limited in the HashMap implementations you can use.<br>
>>> Personally, I don't think it's a memory leak, as GUID number in the system are finite and not too large.<br>
>><br>
>> it's large, it's 128-bit random number, it's not about memory leaking, but cpu cost,<br>
>> as you'll face a lot of rehash'ings in the map,<br>
>><br>
>> i'm not even sure that using indexing in the map can help, worth checking.<br>
>><br>
>> What do you think? Want to add it to your patch?<br>
>><br>
>><br>
>>><br>
>>> On Jun 30, 2013, at 11:13 AM, Yair Zaslavsky wrote:<br>
>>><br>
>>>> Well done, should have been done ages ago :)<br>
>>>> Now, for the painful rebase of async_task_mgr changes :)<br>
>>>><br>
>>>><br>
>>>> ----- Original Message -----<br>
>>>>> From: "Allon Mureinik" <<a href="mailto:amureini@redhat.com">amureini@redhat.com</a>><br>
>>>>> To: "engine-devel" <<a href="mailto:engine-devel@ovirt.org">engine-devel@ovirt.org</a>>, "Barak Azulay" <<a href="mailto:bazulay@redhat.com">bazulay@redhat.com</a>><br>
>>>>> Cc: "Yair Zaslavsky" <<a href="mailto:yzaslavs@redhat.com">yzaslavs@redhat.com</a>>, "Michael Pasternak" <<a href="mailto:mpastern@redhat.com">mpastern@redhat.com</a>>, "Tal Nisan"<br>
>>>>> <<a href="mailto:tnisan@redhat.com">tnisan@redhat.com</a>>, "Ayal Baron" <<a href="mailto:abaron@redhat.com">abaron@redhat.com</a>><br>
>>>>> Sent: Sunday, June 30, 2013 11:11:30 AM<br>
>>>>> Subject: Guid improvements<br>
>>>>><br>
>>>>> Hi all,<br>
>>>>><br>
>>>>> I just merged a couple of improvements to the [N]Guid class [1] to improve<br>
>>>>> it's performance both CPU-wise and memory-wise, based on a set of benchmarks<br>
>>>>> presented by Liran.<br>
>>>>><br>
>>>>> What this patchset achieves:<br>
>>>>> 1. Clean up the code, so it's easier to understand and use<br>
>>>>> 2. Eliminate the inflation in the memory foot print caused by the getValue()<br>
>>>>> method<br>
>>>>> 3. Eliminate all the heavy calls to UUID.fromString when creating a new/empty<br>
>>>>> Guid instance as a default value<br>
>>>>> 4. Note that the cleanups proposed in (1) will have minor performance<br>
>>>>> benefits (e.g., eliminating useless conditional statements), but I doubt<br>
>>>>> this would be anything to write home about.<br>
>>>>><br>
>>>>> From a developer's perspective, here's what changed:<br>
>>>>> 1. No more NGuid, just Guid. Both static methods to create a Guid from String<br>
>>>>> still exist, and are named createGuidFromString and<br>
>>>>> createGuidFromStringDefaultEmpty.<br>
>>>>> 2. [N]Guid.getValue() was removed, it's no longer needed after (1) was<br>
>>>>> implemented<br>
>>>>> 3. The Guid() constructor was made private, as it forced a redundant call to<br>
>>>>> UUID.fromString(String). If you need an empty Guid instance, just use<br>
>>>>> Guid.Empty<br>
>>>>> 4. The Guid.EMPTY_GUID_VALUE string constant was removed, as it was used for<br>
>>>>> redundant calls to UUID.fromString. If you really, REALLY, need it, just<br>
>>>>> call Guid.Empty.getValue() for a UUID or Guid.Empty.toString() for a String.<br>
>>>>> 5. All sorts of ways to transform Strings to Guids were removed. If you have<br>
>>>>> a literal you trust, just use new Guid(String). If you suspect it may be<br>
>>>>> null, use Guid.createGuidFromString[DefaultEmpty]<br>
>>>>> 6. NewGuid is now called newGuid. We're in Java, not C# :-)<br>
>>>>><br>
>>>>><br>
>>>>> Many thanks to everyone who reviewed this patchset.<br>
>>>>> You guys rock!<br>
>>>>><br>
>>>>><br>
>>>>> Regards,<br>
>>>>> Allon<br>
>>>>><br>
>>>>><br>
>>>>> [1]<br>
>>>>> <a href="http://gerrit.ovirt.org/#/q/project:ovirt-engine+branch:master+topic:guid-cleanup,n,z" target="_blank">http://gerrit.ovirt.org/#/q/project:ovirt-engine+branch:master+topic:guid-cleanup,n,z</a><br>
>>>>><br>
>>>> _______________________________________________<br>
>>>> Engine-devel mailing list<br>
>>>> <a href="mailto:Engine-devel@ovirt.org">Engine-devel@ovirt.org</a><br>
>>>> <a href="http://lists.ovirt.org/mailman/listinfo/engine-devel" target="_blank">http://lists.ovirt.org/mailman/listinfo/engine-devel</a><br>
>>><br>
>>> _______________________________________________<br>
>>> Engine-devel mailing list<br>
>>> <a href="mailto:Engine-devel@ovirt.org">Engine-devel@ovirt.org</a><br>
>>> <a href="http://lists.ovirt.org/mailman/listinfo/engine-devel" target="_blank">http://lists.ovirt.org/mailman/listinfo/engine-devel</a><br>
>>><br>
>><br>
>><br>
>> --<br>
>><br>
>> Michael Pasternak<br>
>> RedHat, ENG-Virtualization R&D<br>
><br>
<br>
<br>
--<br>
<br>
Michael Pasternak<br>
RedHat, ENG-Virtualization R&D<br>
</div></div></blockquote></div><br></div>