[Engine-devel] Guid improvements

[Allon Mureinik]

----- Original Message -----
> From: "Liran Zelkha" <liran.zelkha at gmail.com>
> To: "Yair Zaslavsky" <yzaslavs at redhat.com>
> Cc: "Allon Mureinik" <amureini at redhat.com>, "engine-devel" <engine-devel at ovirt.org>
> Sent: Sunday, June 30, 2013 11:37:26 AM
> Subject: Re: [Engine-devel] Guid improvements
> 
> 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.
I generally agree about the high cost of sting<->uuid operations, but I'm not sure caching is the way to go, at least not everywhere.

At least for the database, there is a much easier solution - stop using strings to represent uuids.
Here's an example of how it can be done:
http://gerrit.ovirt.org/#/c/16281/


> 
> 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. 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
> 
>