[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] RFC: Remove all object hashtable caches from virConnectPtr



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.
> 
> Other options include
> 
>  - Explicitly overrite the 'name' field in the cached
>    objects every time they are looked up, to ensure
>    fully up2date objects

This isn't practical actually, because changing the values
of an pre-existing cached virDomainPtr object would not
be threadsafe without holding the global virConnnectPtr
lock when changing it, and in *all* code which reads
it. Strictly speaking this is true even for our code
which changes the 'id' field, but we are luck and get 
away with it, because we don't use very much

>  - Add a flag to virConnectOpen to allow apps to
>    turn off caching themselves.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]