Closed
Bug 631045
Opened 14 years ago
Closed 11 years ago
JM: store PIC code separately from normal code
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
INVALID
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
(Whiteboard: [MemShrink:P3])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•14 years ago
|
||
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 :)
Assignee | ||
Comment 2•14 years ago
|
||
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
Assignee | ||
Comment 3•14 years ago
|
||
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)
Assignee | ||
Updated•14 years ago
|
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.
Assignee | ||
Comment 5•14 years ago
|
||
Same as the last patch, but with the boolean replaced by PoolKind.
Attachment #510902 -
Flags: review?(dvander)
Assignee | ||
Updated•14 years ago
|
Attachment #509328 -
Attachment is obsolete: true
Attachment #509328 -
Flags: review?(dvander)
Updated•14 years ago
|
Attachment #510902 -
Flags: review?(dvander) → review+
Assignee | ||
Updated•14 years ago
|
Blocks: mslim-fx5+
Assignee | ||
Comment 6•13 years ago
|
||
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
Assignee | ||
Updated•13 years ago
|
Whiteboard: [MemShrink:P3]
Comment 7•11 years ago
|
||
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: 11 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•