Re: [naviserver-devel] Excessive locking in urlspace?

[Stephen D. <sd...@gm...>]

You can continue to call ns_unregister_proc.

The comment was for Ns_UrlSpecificDestroy:

    Side effects:
        Will remove data from urlspace; don't call this after server
        startup.

I think this is wrong, so I removed the comment.  If this isn't the
case, I guess we've had a problem all along...

Re compatibility, there are a couple of notes for C module writers in
the patch description.



On 12/9/05, Vlad Seryakov <vl...@cr...> wrote:
> So, that means ns_register_proc i can call, but not ns_unregister_proc?
>
> I have nothing against the patch if no incompabilities will be raised.
>
> Stephen Deasey wrote:
> > Yes, you must still be able to register procs at runtime. (There is a
> > comment in urlspace.c saying you musn't call destroy at runtime...?)
> >
> > I guess my real question was is this the right way to do it?  I
> > originally thought that maybe the urlspace code should hadle all the
> > locking.  I think the idea below is simpler and more flexible.  So
> > here's a patch:
> >
> > http://sourceforge.net/tracker/index.php?func=3Ddetail&aid=3D1377113&gr=
oup_id=3D130646&atid=3D719009
> >
> >
> > The conn pool code is now lock free!
> >
> >
> > On 12/7/05, Vlad Seryakov <vl...@cr...> wrote:
> >
> >>I think ns_register_proc/ns_register_filter uses Trie and canbe called
> >>at any time, that's why it still needs locking.
> >>
> >>Double locking should be avoided but i would keep the ability to call
> >>ns_registerXXX procs at any time.
> >>
> >>Stephen Deasey wrote:
> >>
> >>>Ns_ConnRunRequest in op.c does something like:
> >>>
> >>>Ns_MutexLock(&ulock);
> >>>reqPtr =3D Ns_UrlSpecificGet(server, method, url, id);
> >>>...
> >>>Ns_MutexUnlock(&ulock);
> >>>
> >>>
> >>>Ns_UrlSpecificGet in urlspace.c does something like:
> >>>
> >>>MkSeq(&seq, server, method, url);
> >>>Ns_MutexLock(&lock);
> >>>data =3D JunctionFind(&urlspace, seq, id, 0);
> >>>Ns_MutexUnlock(&lock);
> >>>
> >>>
> >>>i.e. there's double locking going on.  The second lock, in the
> >>>urlspace code, is under contention by all callers of urlspace
> >>>(requests, url2file, etc.), and many of these are very common
> >>>operations.
> >>>
> >>>
> >>>Users of urlspace need to call Ns_UrlSpecificAlloc() in their
> >>>initialisation code to get a magic ID.  This ID partitions the data of
> >>>each subsystem.  But the ID is stored within the 'Trie' and so needs
> >>>to be protected by the global lock.
> >>>
> >>>What if instead the ID was used as in Thread Local Storage?  There's
> >>>an array of slots, where each slot contains a separate Trie, and the
> >>>ID is an index into this array.  Because ID's are allocated at start
> >>>up and aren't destroyed there need be no locking at run time to locate
> >>>the appropriate Trie for the caller.
> >>>
> >>>The urlspace locking can now be removed because the callers locking
> >>>perfectly protects the Trie from concurrent access.  It would also
> >>>allow the caller to run lock-free in the case where all urlspace
> >>>entries are created at start up and only read at run time.
> >>>
> >>>
> >>>Does this make sense?