Closed Bug 631045 Opened 10 years ago Closed 7 years ago

JM: store PIC code separately from normal code

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: njn, Assigned: njn)

References

Details

(Whiteboard: [MemShrink:P3])

Attachments

(1 file, 1 obsolete file)

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.
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 :)
Oh, we do still have to iterate through each PICInfo in order to repatch it.
Summary: JM: avoid fragmentation caused by interleaving normal code and IC code → JM: store PIC code separately from normal code
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.
Attachment #509328 - Flags: review?(dvander)
Blocks: JaegerShrink
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.
Same as the last patch, but with the boolean replaced by PoolKind.
Attachment #510902 - Flags: review?(dvander)
Attachment #509328 - Attachment is obsolete: true
Attachment #509328 - Flags: review?(dvander)
Attachment #510902 - Flags: review?(dvander) → review+
Blocks: 637216
Blocks: mslim-fx5+
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).
Depends on: 661474
Whiteboard: [MemShrink:P3]
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.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.