Investigate if TenuredHeap could be removed

NEW
Unassigned

Status

()

Core
JavaScript: GC
2 years ago
2 years ago

People

(Reporter: smaug, Unassigned)

Tracking

36 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

2 years ago
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.)
(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.
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.
(Reporter)

Comment 3

2 years ago
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.
You need to log in before you can comment on or make changes to this bug.