[Engine-devel] Guid improvements

[Allon Mureinik]

----- Original Message ----- 

> From: "Liran Zelkha" <liran.zelkha at gmail.com>
> To: "Allon Mureinik" <amureini at redhat.com>
> Cc: "Yair Zaslavsky" <yzaslavs at redhat.com>, "engine-devel"
> <engine-devel at ovirt.org>
> Sent: Monday, July 1, 2013 10:36:06 AM
> Subject: Re: [Engine-devel] Guid improvements

> Awesome!!!

> On Mon, Jul 1, 2013 at 10:29 AM, Allon Mureinik < amureini at redhat.com >
> wrote:

> > ----- 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/
This patchset was merged [1].
Along with a couple of uber-neat improvements by Juan [2][3], I think we should see a real boost in performance.

[1] http://gerrit.ovirt.org/#/q/status:merged+project:ovirt-engine+branch:master+topic:guid-database-improvements,n,z
[2] http://gerrit.ovirt.org/#/c/16421/
[3] http://gerrit.ovirt.org/#/c/16467/

> 

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