Closed Bug 663616 Opened 11 years ago Closed 11 years ago

techcrunch produces 20MB of js::Shape, some non-index in-dictionary

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: luke, Unassigned)

References

Details

(Whiteboard: [MemShrink:P3])

A browser with one open tab at techcrunch.com contains 319K shapes (20MB) of which 94% are shapes inDictionary where !JSID_IS_INT(propid).  Summing shapes per compartment and sorting these, 4 compartments contain 80% of the shapes:

 48.4392% (http://www.facebook.com) (like button)
 20.3567% (https://plusone.google.com) (+1 button)
 9.86943% ([System Principal])
 9.50983% (http://techcrunch.com/) 

This seems to indicate that we should try to do something more space efficient for shapes in dictionary mode.  If we just straight-up made a HashMap<jsid,PropStuff>, then with some hackin' it seems like "PropStuff" could only contain a flags/attr word, parent link (to preserve order), and slot index.  That would be a 2-3x space reduction depending on arch.

To wit (this is whole-browser shape usage with 1 or 5 tabs as indicated; dictionary = non-int propid and inDictionary):
 - about:home x1,  2.0MB shapes, 77% dictionary
 - about:home x5,  2.2MB shapes, 81% dictionary
 - facebook x1,    3.4MB shapes, 79% dictionary
 - facebook x5,    7.7MB shapes, 85% dictionary
 - engadget x1,    5.0MB shapes, 81% dictionary
 - engadget x5,   13.8MB shapes, 91% dictionary
 - gmail x1,       5.0MB shapes, 85% dictionary
 - gmail x5,      17.0MB shapes, 92% dictionary
So not as bad as techcrunch, but lots of dictionary shapes and potential for savings.  I'll run my instrumentation on any other sites if anyone would like.
Blocks: mslim-fx5+
Whiteboard: [MemShrink:P2]
Embarrassingly enough, I had a huge bug in the predicate for "dictionary" and the 70-90% figures were way high.  Its more like 17% on techcrunch and 25% on gmail.  Still high, but not about to give us a factor of 2-3x :(
Summary: techcrunch produces 20MB of js::Shape, mostly non-index in-dictionary → techcrunch produces 20MB of js::Shape, much non-index in-dictionary
Another thing I'd like to know about techcrunch.com is why every Facebook "like" button gets its own compartment instead of them all sharing one compartment.  Likewise for all the "+1" buttons.
In comment 0, the facebook.com entry in the table (which held 48% of the shapes) was a single compartment.  Same for google.  So there are an absurd number of compartments, but most of them are mostly empty.
(In reply to comment #3)
... which is still bad since each compartment is ~7K.  Some of the per-compartment overhead (e.g., the 4K traceMonitor vmfragments table) can be hoisted to the JSRuntime with bug 650411.
We don't want to hoist vmfragments into the runtime. Its nice to have it per-origin to avoid cross page threshing. We have to find out why disjoint compartments are created. Is it http or https?
(In reply to comment #5)
> We don't want to hoist vmfragments into the runtime. Its nice to have it
> per-origin to avoid cross page threshing.

Do you have any measurements to indicate that this thrashing would actually be observable in any way to any synthetic or real workload?  For real web workloads, I found in bug 639085 that the tjit is active approximately 0% of the time.  For synthetic workloads, we spend all our time in a single compartment so the issue is moot.
Its well documented in several bugs that in fennec chrome JS and content JS were evicting each other from the code cache.
(In reply to comment #7)
Interesting, I must have missed those bugs flying by.  Questions:
 - Was this post-JM?
 - Can you link bug #s or perhaps just a summary of the actual effect on perf?
I am pretty sure this was pre-JM. The effect was that we run some code in content and then the UI does something (i.e. scroll) and we record there and so on. Note that fennec doesn't run into this problem any more (process separation). Can you explain to me why this problem wouldn't happen with multiple tabs? And what is a real web workload?
(In reply to comment #9)
> I am pretty sure this was pre-JM. The effect was that we run some code in
> content and then the UI does something (i.e. scroll) and we record there and
> so on. Note that fennec doesn't run into this problem any more (process
> separation). Can you explain to me why this problem wouldn't happen with
> multiple tabs?

So, if I'm reading LookupOrAddLoop correctly, even if two unrelated traces map to the same vmfragment location, they both get TreeFragments in the bucket so neither should be knocking out the other's trace.  This leaves (1) ResetJIT calls from one compartment harming unrelated compartments and (2) contention for the precious-few MONITOR_N_GLOBAL_STATES slots.

For (1), we trace a lot less now so it should be less of a problem than in the post-JM trace-or-die era.  For (2), it seems like, if we still observed the problem, we could raise the limit above 4 (with compartment-per-domain it seems like we'd be running into this problem even now).

> And what is a real web workload?

I used a highly subjective word; sorry.  I meant "15 or so common JS-heavy web apps (gmail, zimbra, facebook, etc) and a random sampling of HTML5 games from the Mozilla games competition".  Not a formal study, I know.
(In reply to comment #4)
> (In reply to comment #3)
> ... which is still bad since each compartment is ~7K.

Oh, it's *much* worse than that, at least 250,000 bytes per compartment.  See bug 661068.
Luke: your profiling bug (comment 1) makes the original compressed-dictionary-shapes idea less important.  And the lazy initialization of TraceMonitor (bug 661068) makes the vmfragments side-issue much less important.

In light of that, should we WONTFIXate or INVALIDate this bug?
Whiteboard: [MemShrink:P2] → [MemShrink:P3]
Summary: techcrunch produces 20MB of js::Shape, much non-index in-dictionary → techcrunch produces 20MB of js::Shape, some non-index in-dictionary
You're right; there is no clear action to take other than what we already know we need to do: shrink Shape and do something different for dictionaries.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
Thanks, luke!(

> there is no clear action to take other than what we already
> know we need to do: shrink Shape and do something different for dictionaries.

Do we need to open bugs for those?
You know I haven't really been tracking that, so I don't know; I was just doing some measurements for compartment-per-global.
You need to log in before you can comment on or make changes to this bug.