This will cut the code cache use in half (give or take).
My measurements show it will be more like a 70-80% reduction.
Graydon or I should own this. I will be mostly gone the next 2 weeks, so graydon for now. I will help as needed.
Assignee: general → graydon
This smells a lot like a dupe of 517083, no? If so we should escalate it to P1 blocking, to match this one.
Created attachment 402514 [details] [diff] [review] fix the bug This patch deallocates the LIR after recording, simply by placing it in the tmpAlloc (which is reset post-recording). It also has to clear() the LirBuffer in order to have it forget about the chunk it's part way through carving LIns off. It appears to make sunspider a teensy bit faster, but doesn't do anything positive for the box2d benchmark in bug 479090. Trace stats show 22-23 flushes before the patch, about the same after. Depending on test noise; it varies substantially (sometimes drops down to 4 flushes, then goes fast). What's happening here is that the old "mostly LIR" we were seeing in box2d isn't "mostly LIR" anymore. It was mostly skips. We're no longer keeping pure-LIR around, but we're still getting a lot of *aborts*, and an aborted compilation still generates a lot of non-LIR "persistent" objects in the dataAlloc that are essentially junk because the recording aborted. Fragments, Exits, Guards, etc. etc. They used to be skips so they used to look like LIR, but they never really were. They appear to outweigh the "real" LIR moved by this patch by a substantial margin. I can think of a possible way of fixing this: add a mark/commit/release interface to the allocator. When we start a new recording, mark the start -- save the state of the allocator. When we're sure we want to keep that *increment* of recording-generated material, commit to it. If we abort, release it, rewinding the allocator to the state it was in when we started recording. Currently allocators can't rewind internally, this would make them more like the fancier JS arenas. If you want, I'll open a fresh bug on that.
Attachment #402514 - Flags: review?(gal)
I love #5. Thats exactly what the recorder needs.
(In reply to comment #6) > I love #5. Thats exactly what the recorder needs. Does that mean r+ on the actual attachment? :)
Attachment #402514 - Flags: review?(gal) → review+
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
status1.9.2: --- → beta1-fixed
You need to log in before you can comment on or make changes to this bug.