[Engine-devel] Guid improvements

[Liran Zelkha]

All is true. But average UUID.fromString execution is 1675us, and
HashMap.put is 78us - so the benefit is clear when we're talking on >100K
executions for 10minutes...


On Sun, Jun 30, 2013 at 12:44 PM, Michael Pasternak <mpastern at redhat.com>wrote:

> On 06/30/2013 12:20 PM, Liran Zelkha wrote:
> > I checked such a solution using JProfiler. Creating the GUID object
> takes much more CPU cycles that checking the string in the Map.
>
> of course it is, but what is the size of map that you checked?, check on
> worst scenario, i.e map
> is full of all possible guids,
>
> also problem a bit different,java map has a load factor (which is usually
> 0.75),
> when ratio increases beyond the load factor, occurs proses called re-hash
> so that the hash
> table will double amount of buckets. what can produce a cpu spikes (though
> it should not happen too often),
> to avoid this the initial capacity should be greater than the maximum
> number of entries / the
> load factor, and this is a huge map....
>
> so basically this is a tradeoff between time and space costs against the
> new guid generation.
>
> >
> > 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
> >
>
>
> --
>
> Michael Pasternak
> RedHat, ENG-Virtualization R&D
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/engine-devel/attachments/20130630/e0f5b46d/attachment.html>