Last Comment Bug 661068 - Create TraceMonitors lazily
: Create TraceMonitors lazily
Status: RESOLVED FIXED
[MemShrink:P2][fixed-in-tracemonkey]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal with 4 votes (vote)
: ---
Assigned To: Nicholas Nethercote [:njn]
:
Mentors:
Depends on:
Blocks: mslim-fx5+
  Show dependency treegraph
 
Reported: 2011-05-31 17:37 PDT by Nicholas Nethercote [:njn]
Modified: 2011-06-27 13:25 PDT (History)
24 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
make TraceMonitor lazy (15.80 KB, patch)
2011-06-17 00:11 PDT, Nicholas Nethercote [:njn]
luke: review+
Details | Diff | Splinter Review

Description Nicholas Nethercote [:njn] 2011-05-31 17:37:31 PDT
When a JaegerCompartment is created, it always generates some code immediately in the TrampolineCompiler, which involves mapping 64KB of executable memory (64KB is the minimum code chunk size).

This shouldn't be necessary for atomsCompartment.  It should be easy to add a boolean flag |hasCode| somewhere to JaegerCompartment and avoid this for atomsCompartment.  A small win but an easy one.
Comment 1 Nicholas Nethercote [:njn] 2011-06-01 23:06:01 PDT
We're also allocating 186,000 bytes worth of space for trace JIT data, which is unnecessary for the atomsCompartment.  And probably a bit more for various heap allocations done in TraceMonitor::init().  We should avoid that too.
Comment 2 Nicholas Nethercote [:njn] 2011-06-09 18:32:56 PDT
This idea can be extended.  Some compartments have tiny amounts of JS code in them which never gets method JITted and/or trace JITted (eg. techcrunch.com's front page has dozens of them).  Doing lazy initialization/allocation of the relevant code and data chunks could save quite a bit of space.
Comment 3 Luke Wagner [:luke] 2011-06-12 18:33:19 PDT
Thanks for pointing me to this Nick.  bug 650411 should help a lot since both in theory can be hoisted to the (single-threaded) JSRuntime (which should be very few in number).
Comment 4 Nicholas Nethercote [:njn] 2011-06-12 18:51:37 PDT
(In reply to comment #3)
> Thanks for pointing me to this Nick.  bug 650411 should help a lot since
> both in theory can be hoisted to the (single-threaded) JSRuntime (which
> should be very few in number).

The entire JaegerCompartment and TraceMonitor could be hoisted?  That'd be very nice.
Comment 5 Luke Wagner [:luke] 2011-06-12 23:03:09 PDT
Yep (the comment for JaegerCompartment explains that all it wants is single-threadedness).
Comment 6 Nicholas Nethercote [:njn] 2011-06-16 21:47:25 PDT
Luke, it sounds like a single-threaded JSRuntime won't happen soon.  Assuming that's right, I'll go ahead with this bug, because my per-compartments patch for about:memory shows that it's really easy to get dozens and dozens of tiny compartments, and the thought of wasting 0.25MB on each one is giving me an ulcer.
Comment 7 Nicholas Nethercote [:njn] 2011-06-17 00:11:37 PDT
Created attachment 540002 [details] [diff] [review]
make TraceMonitor lazy

This patch does the TraceMonitor.  Here's an example from about:memory:

│  └──────65,536 B (00.14%) -- compartment(atoms)
│         ├──65,536 B (00.14%) -- mjit-code
│         ├───────0 B (00.00%) -- scripts
│         ├───────0 B (00.00%) -- mjit-data
│         ├───────0 B (00.00%) -- tjit-code
│         └───────0 B (00.00%) -- tjit-data
│                 ├──0 B (00.00%) -- allocators-main
│                 └──0 B (00.00%) -- allocators-reserve

186,000 bytes down, 65,536 to go.  I'll do the patch for the JaegerCompartment on Monday.
Comment 8 Nicholas Nethercote [:njn] 2011-06-17 05:11:02 PDT
(In reply to comment #7)
> 
> 186,000 bytes down

Actually, on 64-bit it's more like 256KB because there are various other things (most notably the TraceNativeStorage) in the TraceMonitor that aren't captured in the memory reporters.  On 32-bit it's a bit less.
Comment 9 Luke Wagner [:luke] 2011-06-17 08:36:19 PDT
Comment on attachment 540002 [details] [diff] [review]
make TraceMonitor lazy

Nice; ulcers are bad.
Comment 10 Nicholas Nethercote [:njn] 2011-06-19 19:01:33 PDT
I've narrowed the focus of this bug just to TraceMonitor;  JaegerCompartment is now covered by bug 665404.
Comment 11 Nicholas Nethercote [:njn] 2011-06-21 04:37:22 PDT
Landed with a follow-up to fix some --disable-tracejit bustage:

http://hg.mozilla.org/tracemonkey/rev/1e464e38591e
http://hg.mozilla.org/tracemonkey/rev/57ef3b619966
Comment 12 Chris Leary [:cdleary] (not checking bugmail) 2011-06-27 11:39:53 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/1e464e38591e
http://hg.mozilla.org/mozilla-central/rev/57ef3b619966
Note: not marking as fixed because fixed-in-tracemonkey is not present on the whiteboard.

Note You need to log in before you can comment on or make changes to this bug.