If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

workaround inefficient reallocation of PL arenas

NEW
Unassigned

Status

NSS
Libraries
P3
enhancement
13 years ago
8 years ago

People

(Reporter: Nelson Bolyard (seldom reads bugmail), Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

In bug 281866, I explain why NSPR's method of recycling arenas wastes
memory, and that it is a problem when CRLs (which tend to be large, 
on the order of several hundred KB) are used in NSS.  

There are several places in NSS that use a PLArenaPool to hold a CRL,
then free it shortly after using it.  The problem of mismatched arena
sizes would be greatly mitigated if NSS would ensure that these large
arenas are always destroyed, not recycled on the free list, regardless
of whether the arena free list is in use or not.

To that end, I propose to create a new function,  PORT_DestroyArena()
that takes the same arguments as PORT_FreeArena() but always calls 
PL_FinishArenaPool() to destroy the arenapool without recycling any
of the arenas in that pool.  Then I propose to replace certain calls 
to PORT_FreeArena with calls to PORT_DestroyArena, particularly for
those that hold CRLs.  

Are there any objections to this plan?

Comment 1

13 years ago
Sounds good to me...

bob

Comment 2

13 years ago
Yes, it's a good idea .
Besides the CRL decoder, I think most of the code that holds CRLs in arenas is
in softoken. I think it might benefit from other improvements as well. I have
seen it do things like copy the entire CRL value from one arena to another, just
over the course of a find objects call.

Comment 3

13 years ago
Added the father of PLArena to the cc list.
Perhaps other copies of PLArena could also
use this improvement.  jsarena.c doesn't
seem to address this issue.

Comment 4

13 years ago
Sorry, added Brendan to the wrong bug.

We might want to name this function
PORT_FinishArena instead to mirror the
PLArena function names.  (I admit PLArena's
function names are not good, but at least
the PORT Arena function names will be
consistent with PLArena's.)

Comment 5

13 years ago
While we are at it, we can simplify the following
code in PORT_FreeArena:

290     if (!pvd) {
291         /* Each of NSPR's DLLs has a function libVersionPoint().
292         ** We could do a lot of extra work to be sure we're calling the
293         ** one in the DLL that holds PR_FreeArenaPool, but instead we
294         ** rely on the fact that ALL NSPR DLLs in the same directory
295         ** must be from the same release, and we call which ever one we get. 
296         */
297         /* no need for thread protection here */
298         pvd = libVersionPoint();
299         if ((pvd->vMajor > 4) || 
300             (pvd->vMajor == 4 && pvd->vMinor > 1) ||
301             (pvd->vMajor == 4 && pvd->vMinor == 1 && pvd->vPatch >= 1)) {
302             const char *ev = PR_GetEnv("NSS_DISABLE_ARENA_FREE_LIST");
303             if (!ev) doFreeArenaPool = PR_TRUE;
304         }
305     }

It's actually wrong to call libVersionPoint()
directly like that.  But since current versions
of NSS can't possibly be used with NSPR releases
before 4.1.1, the NSPR version check can be
simply deleted.

Comment 6

13 years ago
How about something like PORT_FreeArenaNoRecycle
or PORT_ReallyFreeArena?  I won't be able to figure
out the difference between FreeArena, FinishArena,
and DestroyArena just by looking at their names.
So I think my previous suggestion in comment 4 is
a bad idea.
(Reporter)

Comment 7

13 years ago
Putting this bug on my  NSS 3.10 radar.
Priority: -- → P2
Target Milestone: --- → 3.10
(Reporter)

Updated

13 years ago
QA Contact: bishakhabanerjee → jason.m.reid
(Reporter)

Comment 8

13 years ago
Maybe this will happen in 3.11.  
For now, we're finding the best performance by disabling NSS's use of NSPR's
arena free list alltogether and using NSPR's "zone allocator" instead.
Priority: P2 → P3
Target Milestone: 3.10 → 3.11
(Reporter)

Updated

12 years ago
QA Contact: jason.m.reid → libraries
(Reporter)

Comment 9

11 years ago
remove target milestone, since the target was missed.
Target Milestone: 3.11 → ---
(Reporter)

Updated

8 years ago
Assignee: nelson → nobody
You need to log in before you can comment on or make changes to this bug.