631045 - JM: store PIC code separately from normal code

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Normal code and IC code is currently interleaved in the same executable chunks.  But ICs (PICs, at least) get purged on every GC.  This leaves behind holes in the chunks.

Now, the problem isn't too bad because ICs are very small, 10s of bytes, compared to normal code chunks which are 100s or 1000s of bytes.  Furthermore, normal code chunks are discarded if unused for a few minutes.  But it's still probably worth avoiding.

The fix should be straightforward:  store IC code (or maybe just PIC code?) in separate execPools from normal code.

Comment 1

[Nicholas Nethercote [inactive]]

And I just realized that if PIC code is always stored in separate execPools, then purging PICs on GC is much easier -- we can iterate through the PIC execPools rather than iterating through each PICInfo.  Furthermore, each PIC won't need to track its own execPools!

So: simpler, faster and uses less memory :)

Comment 2

[Nicholas Nethercote [inactive]]

Oh, we do still have to iterate through each PICInfo in order to repatch it.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: patch (against TM 61559:a2afd41ae041)

This patch just does the "store PIC code separately from other code" change.  It doesn't remove BasePolyIC::execPools nor free the PIC pools separately.  That's a bigger and riskier change, and one that might benefit from bug 631106 happening first.

dvander, feel free to delegate the review to someone else if you like.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

I really like this idea! Can we have an enum, like { JITPool_Main, JITPool_IC } instead of the boolean parameter? I find boolean parameters to be kind of confusing.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: patch, v2 (against TM 61845:53763d7e9e54)

Same as the last patch, but with the boolean replaced by PoolKind.

Comment 6

[Nicholas Nethercote [inactive]]

Note that this patch will decrease fragmentation, but may end up using more memory because we may end up using more ExecPools.  Eg. in a compartment with a small amount of code we might end up allocating two ExecPools (64KB each) instead of one.  This needs per-compartment measurements for better evaluation (bug 661474).

Comment 7

[Till Schneidereit [:till]]

I don't know what we're doing in ion and baseline, but I hear we have 0 issues with PIC code storage in JM, nowadays.