Re: [Engine-devel] Guid improvements

I checked such a solution using JProfiler. Creating the GUID object takes much more CPU cycles that checking the string in the Map.

On Jun 30, 2013, at 12:06 PM, Michael Pasternak wrote: > On 06/30/2013 11:37 AM, Liran Zelkha wrote: >> Great news. >> 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. >> >> 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. >> Personally, I don't think it's a memory leak, as GUID number in the system are finite and not too large. > > it's large, it's 128-bit random number, it's not about memory leaking, but cpu cost, > as you'll face a lot of rehash'ings in the map, > > i'm not even sure that using indexing in the map can help, worth checking. > > What do you think? Want to add it to your patch? > > >> >> On Jun 30, 2013, at 11:13 AM, Yair Zaslavsky wrote: >> >>> Well done, should have been done ages ago :) >>> Now, for the painful rebase of async_task_mgr changes :) >>> >>> >>> ----- Original Message ----- >>>> From: "Allon Mureinik" <amureini(a)redhat.com&gt; >>>> To: "engine-devel" <engine-devel(a)ovirt.org&gt;, "Barak Azulay" <bazulay(a)redhat.com&gt; >>>> Cc: "Yair Zaslavsky" <yzaslavs(a)redhat.com&gt;, "Michael Pasternak" <mpastern(a)redhat.com&gt;, "Tal Nisan" >>>> <tnisan(a)redhat.com&gt;, "Ayal Baron" <abaron(a)redhat.com&gt; >>>> Sent: Sunday, June 30, 2013 11:11:30 AM >>>> Subject: Guid improvements >>>> >>>> Hi all, >>>> >>>> I just merged a couple of improvements to the [N]Guid class [1] to improve >>>> it's performance both CPU-wise and memory-wise, based on a set of benchmarks >>>> presented by Liran. >>>> >>>> What this patchset achieves: >>>> 1. Clean up the code, so it's easier to understand and use >>>> 2. Eliminate the inflation in the memory foot print caused by the getValue() >>>> method >>>> 3. Eliminate all the heavy calls to UUID.fromString when creating a new/empty >>>> Guid instance as a default value >>>> 4. Note that the cleanups proposed in (1) will have minor performance >>>> benefits (e.g., eliminating useless conditional statements), but I doubt >>>> this would be anything to write home about. >>>> >>>> From a developer's perspective, here's what changed: >>>> 1. No more NGuid, just Guid. Both static methods to create a Guid from String >>>> still exist, and are named createGuidFromString and >>>> createGuidFromStringDefaultEmpty. >>>> 2. [N]Guid.getValue() was removed, it's no longer needed after (1) was >>>> implemented >>>> 3. The Guid() constructor was made private, as it forced a redundant call to >>>> UUID.fromString(String). If you need an empty Guid instance, just use >>>> Guid.Empty >>>> 4. The Guid.EMPTY_GUID_VALUE string constant was removed, as it was used for >>>> redundant calls to UUID.fromString. If you really, REALLY, need it, just >>>> call Guid.Empty.getValue() for a UUID or Guid.Empty.toString() for a String. >>>> 5. All sorts of ways to transform Strings to Guids were removed. If you have >>>> a literal you trust, just use new Guid(String). If you suspect it may be >>>> null, use Guid.createGuidFromString[DefaultEmpty] >>>> 6. NewGuid is now called newGuid. We're in Java, not C# :-) >>>> >>>> >>>> Many thanks to everyone who reviewed this patchset. >>>> You guys rock! >>>> >>>> >>>> Regards, >>>> Allon >>>> >>>> >>>> [1] >>>> http://gerrit.ovirt.org/#/q/project:ovirt-engine+branch:master+topic:guid... >>>> >>> _______________________________________________ >>> Engine-devel mailing list >>> Engine-devel(a)ovirt.org >>> http://lists.ovirt.org/mailman/listinfo/engine-devel >> >> _______________________________________________ >> Engine-devel mailing list >> Engine-devel(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/engine-devel >> > > > -- > > Michael Pasternak > RedHat, ENG-Virtualization R&D