535934 - Add Nelson's newsgroup posting on arenas as comments to secport.c

Status: RESOLVED FIXED

(NSS :: Documentation, defect, P2)

Description

[Wan-Teh Chang]

Attachment: Proposed patch

In a newsgroup posting on 2009-12-17, Nelson wrote a good
description of arenas in reply to Andreev's question
  Re: Should I use SECITEM_AllocItem or PORT_Arena{,Z}Alloc
  memory allocation ?

The attached patch adds Nelson's description as comments to
secport.c.

Andreev, do you think the comments will be more discoverable
if I add them to secport.h instead?

Comment 1

[Konstantin Andreev]

Nice idea, thank you. I believe secport.c is appropriate.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

My description was accurate enough to understand how to use ArenaPools, 
I think, but it is inaccurate in a few details that would be misleading
to someone actually trying to maintain the arenapool code, I think. 
So, if we're going to put it into the code, I should refine it first.

Comment 3

[Konstantin Andreev]

Refinements are usually postponed for indefinite time. I believe that your description can be put into the code "as is", it's better than nothing.

Later, if your time permits, you could refine the description according your taste.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 418477 [details] [diff] [review]
Proposed patch

I'm glad I took the time to review that comment and the code behind it. 
I found that I had misremembered an important detail about thread safety
of marks.  The following comment corrects those mistakes.  

 * ArenaPools are like heaps.  The memory in them consists of large blocks,
 * called arenas, which are allocated from the/a system heap.  Inside an
 * ArenaPool, the arenas are organized as if they were in a stack.  Newly
 * allocated arenas are "pushed" on that stack.  When you attempt to
 * allocate memory from an ArenaPool, the code first looks to see if there
 * is enough unused space in the top arena on the stack to satisfy your
 * request, and if so, your request is satisfied from that arena.
 * Otherwise, a new arena is allocated (or taken from NSPR's list of freed
 * arenas) and pushed on to the stack.  The new arena is always big enough
 * to satisfy the request, and is also at least a minimum size that is
 * established at the time that the ArenaPool is created.
 *
 * The ArenaMark function returns the address of a marker in the arena at
 * the top of the arena stack.  It is the address of the place in the arena
 * on the top of the arena stack from which the next block of memory will
 * be allocated.  Each ArenaPool has its own separate stack, and hence
 * marks are only relevant to the ArenaPool from which they are gotten.
 * Marks may be nested.  That is, a thread can get a mark, and then get
 * another mark.
 *
 * It is intended that all the marks in an ArenaPool may only be owned by a
 * single thread.  In DEBUG builds, this is enforced.  In non-DEBUG builds,
 * it is not.  In DEBUG builds, when a thread gets a mark from an
 * ArenaPool, no other thread may acquire a mark in that ArenaPool while
 * that mark exists, that is, until that mark is unmarked or released.
 * Therefore, it is important that every mark be unmarked or released when
 * the creating thread has no further need for exclusive ownership of the
 * right to manage the ArenaPool.
 *
 * The ArenaUnmark function discards the ArenaMark at the address given,
 * and all marks nested inside that mark (that is, acquired from that same
 * ArenaPool while that mark existed).   It is an error for a thread other
 * than the mark's creator to try to unmark it.  When a thread has unmarked
 * all its marks from an ArenaPool, then another thread is able to set
 * marks in that ArenaPool.  ArenaUnmark does not deallocate (or "pop") any
 * memory allocated from the ArenaPool since the mark was created.
 *
 * ArenaRelease "pops" the stack back to the mark, deallocating all the
 * memory allocated from the arenas in the ArenaPool since that mark was
 * created, and removing any arenas from the ArenaPool that have no
 * remaining active allocations when that is done.  It implicitly releases
 * any marks nested inside the mark being explicitly released.  It is the
 * only operation, other than destroying the arenapool, that potentially
 * reduces the number of arenas on the stack.  Otherwise, the stack grows
 * until the arenapool is destroyed, at which point all the arenas are
 * freed or returned to a "free arena list", depending on their sizes.
 *

Comment 5

[Wan-Teh Chang]

Nelson, could you check in the new comment?  Thanks.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Checking in secport.c; new revision: 1.26; previous revision: 1.25

Done.