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.
Liran, don't get me wrong, i'm not against the caching in general, obviously reads
> writes so
actually i'm all with you, just we're going to significantly enlarge a memory
footprint so i just want
to make sure we're on a right track for the worst scenario where engine runs for ages
and hashmap reaches
it's load factor.
>
> 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 <mpastern(a)redhat.com
<mailto:mpastern@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(a)redhat.com
<mailto:amureini@redhat.com>>
>>>>>>>> To: "engine-devel" <engine-devel(a)ovirt.org
<mailto:engine-devel@ovirt.org>>, "Barak Azulay" <bazulay(a)redhat.com
<mailto:bazulay@redhat.com>>
>>>>>>>> Cc: "Yair Zaslavsky" <yzaslavs(a)redhat.com
<mailto:yzaslavs@redhat.com>>, "Michael Pasternak"
<mpastern(a)redhat.com <mailto:mpastern@redhat.com>>, "Tal Nisan"
>>>>>>>> <tnisan(a)redhat.com
<mailto:tnisan@redhat.com>>, "Ayal Baron" <abaron(a)redhat.com
<mailto:abaron@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...
>>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Engine-devel mailing list
>>>>>>> Engine-devel(a)ovirt.org <mailto:Engine-devel@ovirt.org>
>>>>>>>
http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>>>>
>>>>>> _______________________________________________
>>>>>> Engine-devel mailing list
>>>>>> Engine-devel(a)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
>