1221258 - Investigate if TenuredHeap could be removed

Status: NEW

(Core :: JavaScript: GC, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

It is not clear to my what we actually achieve with TenuredHeap<> other than sort of an annotation. Does it help significantly with performance in any case?


(I started to wonder this while writing a test patch to put more wrappers to nursery.)

Comment 1

[Terrence Cole [:terrence]]

(In reply to Olli Pettay [:smaug] from comment #0)
> It is not clear to my what we actually achieve with TenuredHeap<> other than
> sort of an annotation. Does it help significantly with performance in any
> case?
> 
> 
> (I started to wonder this while writing a test patch to put more wrappers to
> nursery.)

It's actually not for either of those purposes! TenuredHeap is there to support a single tagged pointer instance that needs (probably need*ed* as I can't find a user anymore on dxr) to store 3 bits worth of data in the low bits of the pointer. This is fine with tenured pointers since they are guaranteed to have JS::Value/CellSize alignment, but is /not/ fine with Nursery pointers since those only guarantee pointer alignment -- 2 bits on 32-bit platforms.

The uses in the Cycle Collector at least are only for the assertions; I'd be fine with removing those if we can replace them with nursery usage assertions somehow. The other usages I'm less certain about, but a quick dxr search only shows up two usages in XPConnect, both of which only need one bit.

Comment 2

[Terrence Cole [:terrence]]

Actually, if we still have users that need the tag bits this will still be somewhat difficult to reify as we'd need to make Heap aware of tags or add a second tag-aware buffer. Neither is really appealing. Maybe I'll see how hard it would be to extract the existing tags into a bool member; because of bindings, XPConnect should have a lot less loaded than it was when we landed GGC.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

oh, I wasn't thinking to make any changes to Heap, just make code easier to understand and replace
TenuredHeap with Heap + whatever else is needed.