Last Comment Bug 679942 - attach shareable portion of JSScript to necko cache
: attach shareable portion of JSScript to necko cache
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal with 18 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: Jason Orendorff [:jorendorff]
Depends on: 665707 679940 900255
Blocks: 690229 716323
  Show dependency treegraph
Reported: 2011-08-17 16:56 PDT by Luke Wagner [:luke]
Modified: 2014-10-19 01:22 PDT (History)
37 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

measurement patch (9.48 KB, patch)
2011-08-17 16:56 PDT, Luke Wagner [:luke]
no flags Details | Diff | Splinter Review

Description User image Luke Wagner [:luke] 2011-08-17 16:56:34 PDT
Created attachment 553958 [details] [diff] [review]
measurement patch

bug 665707 proposes adding the ability to attach the "compiled representations" of a resource to its necko cache entry.  The shareable portion of a JSScript would be a prime candidate for this (see bug 665707 comment 8 to 13).

To get a feeling for how much this could save, I instrumented a browser.  On each full GC, the patch maintains a hash set where the key is the tuple (script->filename, script->lineno, script->length, script->code[0..script->length]).  When a script is traced, its key is extracted and it is added to the set.  If there is already a script in the set with the same key, I naively assume the cache would hit (and we'd share the script) and add script->totalSize() to a running sum that is reset/printed on each GC.  Lastly, in view of bug 678037, I only count scripts which have been run once.  This is pretty rough, but I think it gives a nice estimate of how much we could potentially win:

                      # scripts                 totalSize sum
                    total / share / %       total / share  / %
Startup:             1504 /  63   / 4%      765KB / 16.0KB / 2%>14 tabs  29K / 18K   / 63%    16.8MB / 10.4MB / 62%
facebook->5 tabs       7K /  4K   / 64%     3.2MB /  2.0MB / 63%
yahoo->7 tabs         11K /  3K   / 26%     6.3MB /  1.5MB / 24%
wikipedia->20 tabs    10K /  8K   / 85%     6.1MB /  5.3MB / 86%

Here, "Site->N tabs" means "go to Site, then ctrl-click on N things". is interesting because all the tabs are different domains; all the other examples were from the same domain.  I should note that, since not all of JSScript::totalSize() can be shared (some of it needs to stay per-compartment) and, as discussed in bug 678037, most scripts are pretty short, only maybe 60% of the "share" column could actually be saved by caching.  Still, that's a lot.

Also, the compile time for these scripts is a major part of page load time.  On the subject of compile time, bug 288473 has already attempted a caching strategy.  A key difference is that bug 288473 caches XDR (which incurs an encoding penalty) and decodes a new script on each hit (so there is no memory savings).  This mostly results from the use of the existing cache interface instead of the fancy new hotness proposed in bug 665707.

Measurement patch attached.
Comment 1 User image Danial Horton 2012-01-07 19:29:32 PST
Luke, would be interested to test a version of 10 or nightly with this change implemented, theres a massive memory usage in facebook compartments when multiple share or like buttons are active across several tabs
Comment 2 User image Luke Wagner [:luke] 2012-01-08 15:52:36 PST
I'm afraid this isn't close to being implemented.  At the least, it needs support from Necko.
Comment 3 User image Nicholas Nethercote [:njn] 2013-06-13 22:15:57 PDT
Is this still relevant now that bug 679940 has been done?
Comment 4 User image Luke Wagner [:luke] 2013-06-13 22:18:42 PDT
Probably not, I'll WONTFIX for now.
Comment 5 User image Boris Zbarsky [:bz] (still a bit busy) 2013-06-13 22:43:39 PDT
In the current setup, do we just save memory by sharing, or also save compilation time?
Comment 6 User image Till Schneidereit [till] 2013-06-14 04:31:02 PDT
> In the current setup, do we just save memory by sharing, or also save
> compilation time?

The former. Given the results in bug 813324 comment 19, I'm not convinced we should WONTFIX this. There's clearly some value in not only saving memory but also compilation time.

Having said that, other changes might make this moot to some extend. Lazy parsing of scripts will substantially decrease the time needed for parsing in general. Additionally, we might do things like clone scripts from other compartments if they have already been parsed in the current session. That would require at least the hashing part of this task, however.

Given that there's no clearly best path forward here, I think WONTFIXING is premature.
Comment 7 User image Luke Wagner [:luke] 2013-06-14 08:57:30 PDT
Another concern that has come up since filing this: with Till's patch, we always save memory (but never time; we lose a bit of time from the hashing).  If we start caching the bytecode for scripts that aren't being used then we might win some time but we potentially waste a ton of memory.  If we start thinking about saving bytecode to disk, Taras's general feedback on I/O perf indicates that we might end up going slower since we'd be performing more I/O.  That plus lazy parsing is what has made me lose interest in this bug.

OTOH, bug 883154 sounds promising (saves parse time and doesn't waste memory).

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