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

RESOLVED FIXED

Status

()

P2
normal
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: gal, Assigned: graydon)

Tracking

Trunk
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 +
in-testsuite -

Firefox Tracking Flags

(status1.9.2 beta1-fixed, fennec1.0b3+)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

Comment hidden (empty)
(Reporter)

Comment 1

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

Updated

10 years ago
tracking-fennec: --- → 1.0b3+
Depends on: 495734
My measurements show it will be more like a 70-80% reduction.
Flags: blocking1.9.2?

Updated

10 years ago
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P2
(Reporter)

Comment 3

9 years ago
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
(Assignee)

Comment 4

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

Comment 5

9 years ago
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)
(Reporter)

Comment 6

9 years ago
I love #5. Thats exactly what the recorder needs.
(Assignee)

Comment 7

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

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

Updated

9 years ago
Depends on: 518744
(Reporter)

Updated

9 years ago
Attachment #402514 - Flags: review?(gal) → review+
(Assignee)

Comment 8

9 years ago
http://hg.mozilla.org/tracemonkey/rev/80557555961c
Whiteboard: fixed-in-tracemonkey

Comment 9

9 years ago
http://hg.mozilla.org/mozilla-central/rev/80557555961c
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.