1539570 - Improve documentation of Heap<T>, TenuredHeap<T>, HeapPtr<T>, GCPtr<T>

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement, P5)

Description

[Steve Fink [:sfink] [:s:]]

I'm not sure if this should be an overview comment (or a modification of an existing overview comment, eg the one in gc/Barrier.h) or changes to existing comments before each of the types.

The sort of thing I'm looking for is that it seems like each class handles a different subset of

• prebarriered?
• postbarriered?
  • movable?
  • destructible outside of a GC?
• readbarriered? (sort of separately for weak or CC-gray reasons)

and perhaps things I'm missing. (Scanning through, I notice a bunch of less interesting ones: alignment, flag bits of TenuredHeap<T>, ...)

Specific information I find lacking:

• Heap<T> is not pre-barriered, and why that is ok.
• why to use HeapPtr<T> within the engine instead of Heap<T>
• that Rooted<T> does not need write barriers of either sort, and why
• what types to use as fields of a stack-only class

Some of this is just me -- every time I reread the existing documentation, I discover that something I thought was missing is actually there, quite plainly.

Comment 1

[Paul Bone [:pbone]]

I was going to respond and say "I've needinfo'd sfink, every 6 months or so he writes 80% of a new document about this". Then noticed who was reporting this.

We should update the wiki page also. <- actually constructive comment.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: bug1539570-barrier-do

Here's a first attempt to improve the comments. How does this look? Any areas that need improving?

Comment 3

[Steve Fink [:sfink] [:s:]]

Comment on attachment 9054183 [details] [diff] [review]
bug1539570-barrier-do

Review of attachment 9054183 [details] [diff] [review]:
-----------------------------------------------------------------

Oh, sorry! I hadn't seen this patch at the time of our meeting. This is great! It answers the main questions I always have right up front.

I have a couple of minor nits, but I'll leave those for the review.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1539570 - Improve documentation of our various barrier classes r=sfink?

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/48aa3ad3d66e
Improve documentation of our various barrier classes r=sfink

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/48aa3ad3d66e