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?
Dave