[Engine-devel] Guid & NGuid

Tue Feb 5 07:28:26 UTC 2013


----- 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.

> 
> Regards,
> Alon
>