Open Bug 476540 Opened 16 years ago Updated 2 years ago

PRLock and PRMonitor should not force heap allocation

Categories

(NSPR :: NSPR, enhancement)

Product:
NSPR
Component:
NSPR
Type:
enhancement

Tracking

(Not tracked)

People

(Reporter: roc, Unassigned)

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?
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.

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: wtc → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.