PRLock and PRMonitor should not force heap allocation

NEW
Assigned to

Status

NSPR
NSPR
--
enhancement
9 years ago
9 years ago

People

(Reporter: roc, Assigned: Wan-Teh Chang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Right now the only way to use these objects is via a heap allocation. It would be more efficient and more convenient to allow these objects to be direct members of other objects. Looking at primpl.h this doesn't look like it would be hard to do.
This is equivalent to a request for a new API for initializing pre-allocated
locks, and for being smart about destroying them (I suppose).  

One issue is that implementing this makes those locks no longer completely
opaque, because the size becomes known and fixed thereafter.  This is a 
concern for binary compatibility, going forward.
Severity: normal → enhancement
pthreads does this, Windows does it, why can't we do it?
(Assignee)

Comment 3

9 years ago
roc: Yes, we can do it.  We'll have to either freeze the PRLock
structure (used by CRITICAL_SECTION on Windows) or define a public
PRLock structure of the right size and alignment with dummy members
and cast it to the real PRLock structure on entry to NSPR lock
functions (used by pthreads on Solaris, if I remember correctly).

I now maintain NSPR in my spare time, often on weekends.  To ensure
that this work gets done, reviewed, and checked in to your satisfaction,
could you nominate a Mozilla developer familiar with system programming
to be an NSPR module owner or peer?  I don't want to be the bottleneck
of your progress, and I welcome new blood who wants to tackle challenging multithreading problems.
Maybe Chris would like to take the lead on this.
(In reply to comment #4)
> Maybe Chris would like to take the lead on this.
If not, I can probably do this.
Shawn: please feel free to do this.  I'll be busy finishing up bug 58904 and related issues for a while.  Please keep in mind that after bug 58904, using PR_Lock directly will be deprecated, so your fix will probably need integrate into the |nsLock| class from that patch.  (That won't be a problem at all.)
I'll see if I can tackle this.  I'd love to get rid of some lock allocations I do in storage.

Now I just need to figure out how to checkout and build standalone NSPR so I can generate a useful patch :)
Assignee: wtc → sdwilsh
I've got more important stuff that has come up, so it'll be a while before I could get to this.  Someone else feel free to take it!
Assignee: sdwilsh → wtc
mozilla::Mutex and friends can now be non-heap-allocated.  See bug 58904.

So this change is no longer necessary for Mozilla code cleanliness, although we may want to revisit it as an optimization.
You need to log in before you can comment on or make changes to this bug.