On 06/30/2013 12:20 PM, Liran Zelkha wrote:of course it is, but what is the size of map that you checked?, check on worst scenario, i.e map
> I checked such a solution using JProfiler. Creating the GUID object takes much more CPU cycles that checking the string in the 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@redhat.com>
>>>>> To: "engine-devel" <engine-devel@ovirt.org>, "Barak Azulay" <bazulay@redhat.com>
>>>>> Cc: "Yair Zaslavsky" <yzaslavs@redhat.com>, "Michael Pasternak" <mpastern@redhat.com>, "Tal Nisan"
>>>>> <tnisan@redhat.com>, "Ayal Baron" <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-cleanup,n,z
>>>>>
>>>> _______________________________________________
>>>> Engine-devel mailing list
>>>> Engine-devel@ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>
>>> _______________________________________________
>>> Engine-devel mailing list
>>> 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