On 06/30/2013 01:08 PM, Liran Zelkha wrote: > Why synchronization? No need for it. Worst case scenario a put (which should > be much less common then get) will occur twice on the same key.
why assuming a best & not worst scenario? don't forget that every new insertion requires collision resolution which is triggers .equals() on the GUID. > > On Jun 30, 2013, at 1:00 PM, Michael Pasternak wrote: > >> 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 > -- Michael Pasternak RedHat, ENG-Virtualization R&D _______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel