naviserver-devel

[naviserver-devel] Excessive locking in urlspace?

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

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?

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

[Vlad S. <vl...@cr...>]

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 = 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 = 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?
> 
> 
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> for problems?  Stop!  Download the new AJAX search engine that makes
> searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> http://ads.osdn.com/?ad_idv37&alloc_id865&op=click
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel
> 

-- 
Vlad Seryakov
571 262-8608 office
vl...@cr...
http://www.crystalballinc.com/vlad/

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

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

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&group_=
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?

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

[Vlad S. <vl...@cr...>]

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=detail&aid=1377113&group_id=130646&atid=719009
> 
> 
> 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 = 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 = 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?
> 
> 
> 
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> for problems?  Stop!  Download the new AJAX search engine that makes
> searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> http://ads.osdn.com/?ad_idv37&alloc_id865&op=click
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel
> 

-- 
Vlad Seryakov
571 262-8608 office
vl...@cr...
http://www.crystalballinc.com/vlad/

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?