[Engine-devel] Guid & NGuid

Sun Feb 10 20:28:06 UTC 2013


On 02/10/2013 02:26 PM, Gilad Chaplik wrote:
> ----- Original Message -----
>> From: "Alon Bar-Lev"<alonbl at redhat.com>
>> To: "Michael Kublin"<mkublin at redhat.com>
>> Cc: engine-devel at ovirt.org
>> Sent: Tuesday, February 5, 2013 9:29:20 AM
>> Subject: Re: [Engine-devel] Guid&  NGuid
>>
>>
>>
>> ----- Original Message -----
>>> From: "Michael Kublin"<mkublin at redhat.com>
>>> To: "Alon Bar-Lev"<alonbl at redhat.com>
>>> Cc: engine-devel at ovirt.org
>>> Sent: Tuesday, February 5, 2013 9:28:26 AM
>>> Subject: Re: [Engine-devel] Guid&  NGuid
>>>
>>>
>>>
>>> ----- Original Message -----
>>>> From: "Alon Bar-Lev"<alonbl at redhat.com>
>>>> To: "Michael Kublin"<mkublin at redhat.com>
>>>> Cc: engine-devel at ovirt.org
>>>> Sent: Monday, February 4, 2013 4:17:39 PM
>>>> Subject: Re: [Engine-devel] Guid&  NGuid
>>>>
>>>>
>>>>
>>>> ----- Original Message -----
>>>>> From: "Michael Kublin"<mkublin at redhat.com>
>>>>> To: engine-devel at ovirt.org
>>>>> Sent: Monday, February 4, 2013 2:15:40 PM
>>>>> Subject: Re: [Engine-devel] Guid&  NGuid
>>>>>
>>>>>
>>>>>
>>>>> ----- Original Message -----
>>>>>> From: "Mike Kolesnik"<mkolesni at redhat.com>
>>>>>> To: "Michael Kublin"<mkublin at redhat.com>
>>>>>> Cc: engine-devel at ovirt.org
>>>>>> Sent: Monday, February 4, 2013 11:48:45 AM
>>>>>> Subject: Re: [Engine-devel] Guid&  NGuid
>>>>>>
>>>>>> ----- Original Message -----
>>>>>>>
>>>>>>> ----- Original Message -----
>>>>>>>> From: "Laszlo Hornyak"<lhornyak at redhat.com>
>>>>>>>> To: "Michael Kublin"<mkublin at redhat.com>
>>>>>>>> Cc: engine-devel at ovirt.org
>>>>>>>> Sent: Monday, February 4, 2013 9:56:28 AM
>>>>>>>> Subject: Re: [Engine-devel] Guid&  NGuid
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ----- Original Message -----
>>>>>>>>> From: "Michael Kublin"<mkublin at redhat.com>
>>>>>>>>> To: engine-devel at ovirt.org
>>>>>>>>> Sent: Monday, February 4, 2013 8:47:29 AM
>>>>>>>>> Subject: Re: [Engine-devel] Guid&  NGuid
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ----- Original Message -----
>>>>>>>>>> From: "Roy Golan"<rgolan at redhat.com>
>>>>>>>>>> To: engine-devel at ovirt.org
>>>>>>>>>> Sent: Monday, February 4, 2013 9:18:21 AM
>>>>>>>>>> Subject: Re: [Engine-devel] Guid&  NGuid
>>>>>>>>>>
>>>>>>>>>> On 02/03/2013 03:19 PM, Alon Bar-Lev wrote:
>>>>>>>>>>> ----- Original Message -----
>>>>>>>>>>>> From: "Omer Frenkel"<ofrenkel at redhat.com>
>>>>>>>>>>>> To: "Michael Kublin"<mkublin at redhat.com>
>>>>>>>>>>>> Cc: "engine-devel"<engine-devel at ovirt.org>
>>>>>>>>>>>> Sent: Sunday, February 3, 2013 3:12:19 PM
>>>>>>>>>>>> Subject: Re: [Engine-devel] Guid&  NGuid
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> ----- Original Message -----
>>>>>>>>>>>>> From: "Michael Kublin"<mkublin at redhat.com>
>>>>>>>>>>>>> To: "engine-devel"<engine-devel at ovirt.org>
>>>>>>>>>>>>> Sent: Sunday, February 3, 2013 3:10:14 PM
>>>>>>>>>>>>> Subject: [Engine-devel] Guid&  NGuid
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> In ovirt-engine code we have Guid and NGuid
>>>>>>>>>>>>> objects.
>>>>>>>>>>>>> Guid is extends NGuid and also NGuid class has
>>>>>>>>>>>>> method
>>>>>>>>>>>>> getValue()
>>>>>>>>>>>>> which should return Guid.
>>>>>>>>>>>>> As for me these two classes are look like the
>>>>>>>>>>>>> same
>>>>>>>>>>>>> and
>>>>>>>>>>>>> I
>>>>>>>>>>>>> don't
>>>>>>>>>>>>> see
>>>>>>>>>>>>> to
>>>>>>>>>>>>> much differences between them.
>>>>>>>>>>>>> My proposal is to remove NGuid and move it
>>>>>>>>>>>>> functionality
>>>>>>>>>>>>> to
>>>>>>>>>>>>> Guid
>>>>>>>>>>>>> (Because of Guid is much more common)
>>>>>>>>>>>>>
>>>>>>>>>>>> i agree, but we need to take another step forward
>>>>>>>>>>>> and
>>>>>>>>>>>> allow
>>>>>>>>>>>> Guid
>>>>>>>>>>>> to
>>>>>>>>>>>> be null (as it should)
>>>>>>>>>>>> and not assume its EMPTY or have a value (i'm
>>>>>>>>>>>> pretty
>>>>>>>>>>>> sure
>>>>>>>>>>>> we
>>>>>>>>>>>> have
>>>>>>>>>>>> this assumption in many places)
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> And for the new people out here... why not kill
>>>>>>>>>>> both
>>>>>>>>>>> and
>>>>>>>>>>> use
>>>>>>>>>>> plain
>>>>>>>>>>> standard java UUID[1]?
>>>>>>>>>> +1 for using java.util.UUID
>>>>>>>>>>
>>>>>>>>>> NGuid functionality that should be extracted during
>>>>>>>>>> the
>>>>>>>>>> refactor
>>>>>>>>>> is
>>>>>>>>>> -
>>>>>>>>>> 1. refactor DB and Java so empty or null return
>>>>>>>>>> values
>>>>>>>>>> are
>>>>>>>>>> null
>>>>>>>>>> and
>>>>>>>>>> not
>>>>>>>>>> EMPTY_GUID
>>>>>>>>>> 2. the special constructor of NGuid for UUID return
>>>>>>>>>> by
>>>>>>>>>> Microsoft
>>>>>>>>>> AD
>>>>>>>>>> should be extracted to a factory/utility
>>>>>>>>>>> I think we should kill compat, I don't see any
>>>>>>>>>>> value
>>>>>>>>>>> in
>>>>>>>>>>> fixing
>>>>>>>>>>> anything about it while leaving it intact.
>>>>>>>>>>>
>>>>>>>>>>> Regards,
>>>>>>>>>>> Alon Bar-Lev.
>>>>>>>>>>>
>>>>>>>>>>> [1]
>>>>>>>>>>> http://docs.oracle.com/javase/6/docs/api/java/util/UUID.html
>>>>>>>>> Actually there is exists one reason not to use directly
>>>>>>>>> UUID
>>>>>>>>> at
>>>>>>>>> server side.
>>>>>>>>> The main operations on Guid today it is to convert
>>>>>>>>> object
>>>>>>>>> to
>>>>>>>>> Guid
>>>>>>>>> or
>>>>>>>>> Guid to string.
>>>>>>>>> Guid it is immutable object, number of Guids is limited
>>>>>>>>> and
>>>>>>>>> almost
>>>>>>>>> never changed,
>>>>>>>>> These sound like classical case for object that can be
>>>>>>>>> cached.
>>>>>>>>> Benefit - reduced number of string manipulations,
>>>>>>>>> reduced
>>>>>>>>> number
>>>>>>>>> of
>>>>>>>>> created instances, less work
>>>>>>>>> for garbage collection
>>>>>>>> UUID has the same properties, it is immutable. So you
>>>>>>>> could
>>>>>>>> do
>>>>>>>> the
>>>>>>>> same with UUID, but I am not sure you could effectively
>>>>>>>> cache
>>>>>>>> these
>>>>>>>> objects and prove that you are saving either CPU, heap,
>>>>>>>> or
>>>>>>>> GC
>>>>>>>> time.
>>>>>>> These is a very common pattern:
>>>>>>> 1) implementation of valueOf() for most classes like
>>>>>>> Integer,
>>>>>>> Double,
>>>>>>> etc... has some kind of cache.
>>>>>>> 2) JVM has cache for strings that can be used and that
>>>>>>> cache
>>>>>>> can
>>>>>>> be
>>>>>>> tweaked by some JVM opts.
>>>>>>> 3) Most of our operations is perform query on DB, send
>>>>>>> request
>>>>>>> to
>>>>>>> host or parse response from host and Guid
>>>>>>>     is very common object that is immutable.
>>>>>>> I am agree that these will not solve all our performance
>>>>>>> problems
>>>>>>> but
>>>>>>> can provide some benefit, especially when it is very easy
>>>>>>> to implement.
>>>>>> We could still achieve that using a UUIDCreator class and
>>>>>> call
>>>>>> it
>>>>>> instead of Guid.fromString("")..
>>>>>>
>>>>>> Whether this is cached or not is another question which can
>>>>>> be
>>>>>> solved
>>>>>> later and checked to see if the cache improves performance or
>>>>>> not.
>>>>> These is already implementation, if it will be
>>>>> Guid.fromString("")
>>>>> or
>>>>> UUIDCreator.
>>>>> The issue is if we want to use cache, it should be implemented
>>>>> together
>>>>> with deleting/replacing of Guid/NGuid , because of it much more
>>>>> easilly
>>>> I think the value of using standard java classes is higher than
>>>> the
>>>> tuning of the engine at this regard. Dropping the compat thing is
>>>> important activity.
>>> Immutable object it is a common java design pattern
>>>> After doing this conversion, use profiler to determine where
>>>> major
>>> Already was done, and already tested a cache solution (Simple POC
>>> solution took to implement less
>>> than hour)
>>>> bottle necks are and fix these, I am not sure the above
>>>> optimization
>>>> will be first in list.
>>> Any optimization can not be first in the list, the best
>>> optimization
>>> is architecture change
>>>> If we invest resources in optimization better
>>>> to invest in these that we suffer most.
>>> Most of the issues are well known.
>> Please try to be less cryptic... Is this one of them?
> Another issue, look at:
> http://www.2ality.com/2009/01/uuids-for-gwt.html
> GWT must have a wrapper for uuid, if removing Guid, we must override UUID in GWT.
Not a serious issue, we have a uioverrides package anyway, we can make a 
wrapper in GWT
> I'm +1 on leaving Guid for now.
>
> IMHO the bigger problem we have in the Guid area is
> the ambiguous use of Guid.Empty/null; for me Guid.Empty is BLANK_TEMPLTAE_UUID.
That can indeed be an issue, there are thousands of references in the 
project to the wrapper classes, changing it at once might cause us NPEs 
from here to eternity
>>>> Regards,
>>>> Alon
>>>>
>> _______________________________________________
>> 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