1710394 - Consider removing JitScript's IC stub space

Status: RESOLVED DUPLICATE of bug 1863939

(Core :: JavaScript Engine: JIT, task, P3)

Description

[Jan de Mooij [:jandem]]

After bug 1710075, Baseline IC fallback stubs are no longer LifoAlloc-allocated. This means the stub space (a LifoAlloc) stored in JitScript is only used for CacheIR stubs that can trigger GC. All other CacheIR stubs are allocated with the per-zone allocator (and can be purged on GC).

I think it now becomes feasible to remove the per-JitScript LifoAlloc entirely and allocate all CacheIR stubs with the per-zone allocator. This would require a mechanism to clone Baseline CacheIR stubs that are active on the stack (BaselineStub frames already contain a pointer to them for GC reasons). Probably with a forwarding mechanism so each stub is cloned at most once.

Benefits would be:

• Less malloc pressure and wasted space: especially for small scripts, the per-script LifoAlloc with chunks of 4096 bytes is pretty inefficient because it will be mostly unused.
• Potentially better cache locality when using a single bump allocator.
• More IC stubs for scripts active on the stack can be discarded. This is nice for low memory situations because it potentially lets us collect more IC code, shapes, etc.

Comment 1

[Ted Campbell [:tcampbell]]

We found that with Stencil that 512B LifoAlloc chunks gave us better performance overall than 4096B. If you don't end up removing the LifoAlloc in the short term, shrinking the chunk size is probably benficial.

Comment 2

[Jan de Mooij [:jandem]]

Iain and I discussed this in the Warp call. We realized there's an alternative solution that might be simpler to implement: for each ICStub active on the stack, we mark its LifoAlloc chunk as "in use", and transfer just these chunks to the new LifoAlloc. Then all IC stubs that are not on the stack can be discarded. This keeps more memory around, but is simpler and would get us most of the benefits.

For this to work efficiently, we'd ideally be able to go from ICStub* to its containing BumpChunk in O(1) time, but there are probably other ways to implement this.

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

This is done? I think?

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #3)

This is done? I think?

Yep, bug 1863939. I think I forgot we already had this on file.