[Engine-devel] Guid improvements

[Liran Zelkha]

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 at redhat.com>
>>>> To: "engine-devel" <engine-devel at ovirt.org>, "Barak Azulay" <bazulay at redhat.com>
>>>> Cc: "Yair Zaslavsky" <yzaslavs at redhat.com>, "Michael Pasternak" <mpastern at redhat.com>, "Tal Nisan"
>>>> <tnisan at redhat.com>, "Ayal Baron" <abaron at redhat.com>
>>>> 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-cleanup,n,z
>>>> 
>>> _______________________________________________
>>> Engine-devel mailing list
>>> Engine-devel at ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>> 
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>> 
> 
> 
> -- 
> 
> Michael Pasternak
> RedHat, ENG-Virtualization R&D