497009 - TM: deallocate LIR buffer after compilation if we don't plan on recompiling a tree

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Comment 1

[Andreas Gal :gal]

This will cut the code cache use in half (give or take).

Comment 2

[David Mandelin [:dmandelin]]

My measurements show it will be more like a 70-80% reduction.

Comment 3

[Andreas Gal :gal]

Graydon or I should own this. I will be mostly gone the next 2 weeks, so graydon for now. I will help as needed.

Comment 4

[Graydon Hoare :graydon]

This smells a lot like a dupe of 517083, no? If so we should escalate it to P1 blocking, to match this one.

Comment 5

[Graydon Hoare :graydon]

Attachment: 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.

Comment 6

[Andreas Gal :gal]

I love #5. Thats exactly what the recorder needs.

Comment 7

[Graydon Hoare :graydon]

(In reply to comment #6)
> I love #5. Thats exactly what the recorder needs.

Does that mean r+ on the actual attachment? :)

Comment 8

[Graydon Hoare :graydon]

http://hg.mozilla.org/tracemonkey/rev/80557555961c

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/80557555961c

Comment 10

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2a46ce3befcf