On 06/30/2013 12:45 PM, Liran Zelkha wrote: > 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...
even with synchronization? what about ConcurrentHashMap? > > > On Sun, Jun 30, 2013 at 12:44 PM, Michael Pasternak <mpast...@redhat.com > <mailto:mpast...@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" <amure...@redhat.com > <mailto:amure...@redhat.com>> > >>>>> To: "engine-devel" <engine-devel@ovirt.org > <mailto:engine-devel@ovirt.org>>, "Barak Azulay" <bazu...@redhat.com > <mailto:bazu...@redhat.com>> > >>>>> Cc: "Yair Zaslavsky" <yzasl...@redhat.com > <mailto:yzasl...@redhat.com>>, "Michael Pasternak" <mpast...@redhat.com > <mailto:mpast...@redhat.com>>, "Tal Nisan" > >>>>> <tni...@redhat.com <mailto:tni...@redhat.com>>, "Ayal Baron" > <aba...@redhat.com <mailto:aba...@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@ovirt.org <mailto:Engine-devel@ovirt.org> > >>>> http://lists.ovirt.org/mailman/listinfo/engine-devel > >>> > >>> _______________________________________________ > >>> Engine-devel mailing list > >>> Engine-devel@ovirt.org <mailto:Engine-devel@ovirt.org> > >>> http://lists.ovirt.org/mailman/listinfo/engine-devel > >>> > >> > >> > >> -- > >> > >> Michael Pasternak > >> RedHat, ENG-Virtualization R&D > > > > > -- > > Michael Pasternak > RedHat, ENG-Virtualization R&D > > -- Michael Pasternak RedHat, ENG-Virtualization R&D _______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
