On Thu, Nov 11, 2010 at 04:55:03PM +0000, Daniel P. Berrange wrote: > On Thu, Nov 11, 2010 at 11:44:42AM -0500, Dave Allan wrote: > > On Thu, Nov 11, 2010 at 04:20:43PM +0000, Daniel P. Berrange wrote: > > > On Fri, Oct 29, 2010 at 12:52:18PM +0100, Daniel P. Berrange wrote: > > > > The virConnectPtr struct will cache instances of all other > > > > objects. APIs like virDomainLookupByUUID will return a > > > > cached object, so if you do virDomainLookupByUUID twice in > > > > a row, you'll get the same exact virDomainPtr instance. > > > > > > > > This does not have any performance benefit, since the actual > > > > logic in virDomainLookupByUUID (and other APIs returning > > > > virDomainPtr, etc instances) is not short-circuited. All > > > > it does is to ensure there is only one single virDomainPtr > > > > in existance for any given UUID. > > > > > > > > The caching has a number of downsides though, all relating > > > > to stale data. If APIs aren't careful to always overwrite > > > > the 'id' field in virDomainPtr it may become out of data. > > > > Likewise for the name field, if a guest is renamed, or if > > > > a guest is deleted, and then a new one created with the > > > > same UUID but different name. > > > > > > > > This has been an ongoing, endless source of bugs for all > > > > applications using libvirt from languages with garbage > > > > collection, causing them to get virDomainPtr instances > > > > from long ago with stale data. > > > > > > > > The caching is also a waste of memory resources, since > > > > both applications, and language bindings often maintain > > > > their own hashtable caches of object instances. > > > > > > > > This patch removes all the hash table caching, so all > > > > APIs return brand new virDomainPtr (etc) object instances. > > > > > > No one has any comments on this change I thought would be > > > hugely controversial... ? > > > > From what you've decribed, it seems like a good change. I would have > > thought that there would be a performance hit, but from what you've > > said that's not the case. Why did you think it would be > > controversial? > > Technically it is a semantic API change, because you can no longer rely > on two virDomainPtr for the same domain having the same pointer address. > I think this is pretty unlikely to hurt anything, and any app relying > on this can be argued to be broken, but you never know.... Ah, now I see. I agree with your assessment. I haven't reviewed the code, but ACK to the design. Dave -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list