[Engine-devel] Guid improvements

[Michael Pasternak]

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