On Sun, Jul 23, 2017 at 02:33:49PM +0200, Michal Privoznik wrote: > On 07/23/2017 02:10 PM, John Ferlan wrote: > > > > > > On 07/19/2017 10:31 AM, Michal Privoznik wrote: > >> While this is not that critical (hash tables have O(1) time complexity for > >> lookups), it lays down path towards making virDomainObj using RW locks > >> instead > >> of mutexes. Still, in my limited testing this showed slight improvement. > >> > >> Michal Privoznik (3): > >> virthread: Introduce virRWLockInitPreferWriter > >> virobject: Introduce virObjectRWLockable > >> virdomainobjlist: Use virObjectRWLockable > >> > >> src/conf/virdomainobjlist.c | 24 ++++---- > >> src/libvirt_private.syms | 4 ++ > >> src/util/virobject.c | 144 > >> ++++++++++++++++++++++++++++++++++---------- > >> src/util/virobject.h | 16 +++++ > >> src/util/virthread.c | 35 +++++++++++ > >> src/util/virthread.h | 1 + > >> 6 files changed, 180 insertions(+), 44 deletions(-) > >> > > > > This could be a "next step" in the work I've been doing towards a common > > object: > > Sure. If we have just one common object the change can be done in one > place instead of many. I don't care in what order are the changes merged.
I'm still not sure about the implementation that you are heading to, I would actually prefer something similar to the current virDomainObjList implementation, create a new module in utils called virObjectList and make it somehow generic that it could be used by our objects. I personally don't like the fact that there will be two new classes, one that enables using the other one. > > > > https://www.redhat.com/archives/libvir-list/2017-June/msg00916.html > > > > which moves all those driver/vir*obj list API's for Lookup, Search, > > ForEach, Add, Remove, etc. into object code since they're essentially > > all following the same pattern. > > > > Once there - altering the Lockable lock under the covers should be > > relatively simple. I would be called a ListLock or HashLock instead of > > an RWLock and the implementation is such that it's R or W depending on > > API. Taking a quick refresher look at the series, for a new object I > > call virObjectLookupHashClass - that could be the integration point to > > use a local initialization for the class and then the appropriate lock > > style depending on API. > > I think I still prefer "RWLock" name over "ListLock" or "HashLock" since > its name clearly suggests its purpose. I mean, ListLock doesn't say it's > an RW lock. Moreover, as I say in the cover letter, I'd like to use RW > locks for virDomainObj one day (for instance, there's no reason why two > clients cannot fetch XML for the same domain at the same time). > Therefore, it looks correct to me to derive virDomainObjClass from > virObjectRWLockable instead of ListLock or HashLock or something. I agree that RWLock is more descriptive about what it is. And I also agree that deriving virDomainObjClass from virObjectRWLockable is better. As I've already wrote above, the generic listing code should work without a need to somehow modify the existing objects. Just a note, I like the idea that there will be only one implementation for listing object that will be reused, however the current proposed implementation isn't that convincing to me. > > > > Just thinking off the cuff and of course trying to keep stuff I've been > > working on fresh ;-) > > Sure, the more recent some patches are the more I understand them. Same > here :-) I think that this can be easily merged before the work for listing object gets finished since it can be used for object that doesn't require listing. Pavel
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
