Last Comment Bug 648125 - Allow caching JS loaded with loadSubScript
: Allow caching JS loaded with loadSubScript
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla8
Assigned To: Michael Wu [:mwu]
:
Mentors:
: 648124 (view as bug list)
Depends on: 481603 592943 736519 821726
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-06 14:40 PDT by Dave Townsend [:mossop]
Modified: 2013-08-05 04:27 PDT (History)
10 users (show)
mounir: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP: Cache subscripts in startupcache (16.30 KB, patch)
2011-04-07 16:23 PDT, Michael Wu [:mwu]
no flags Details | Diff | Review
Cache subscripts in startupcache (35.45 KB, patch)
2011-04-19 16:39 PDT, Michael Wu [:mwu]
no flags Details | Diff | Review
Cache subscripts in startupcache, v2 (28.60 KB, patch)
2011-06-10 20:24 PDT, Michael Wu [:mwu]
no flags Details | Diff | Review
Cache subscripts in startupcache, v3 (26.30 KB, patch)
2011-06-13 20:37 PDT, Michael Wu [:mwu]
no flags Details | Diff | Review
Cache subscripts in startupcache, v4 (26.31 KB, patch)
2011-06-14 19:53 PDT, Michael Wu [:mwu]
no flags Details | Diff | Review
Cache subscripts in startupcache, v5 (25.91 KB, patch)
2011-06-29 19:38 PDT, Michael Wu [:mwu]
jst: review+
Details | Diff | Review

Description Dave Townsend [:mossop] 2011-04-06 14:40:57 PDT
Either directly or with a new API we could do with a means of caching JS loaded through loadSubScript, see bug 648124 for a prime use.
Comment 1 Michael Wu [:mwu] 2011-04-07 16:23:18 PDT
Created attachment 524525 [details] [diff] [review]
WIP: Cache subscripts in startupcache

Patch based on tracemonkey tip. Needs a bit more cleanup but it should work if someone wants to take this and measure how much time we save on bootstrap.js when cached.
Comment 2 Michael Wu [:mwu] 2011-04-19 16:39:11 PDT
Created attachment 527150 [details] [diff] [review]
Cache subscripts in startupcache

This one passes tests.
Comment 3 Kris Maglione [:kmag] 2011-06-10 19:01:10 PDT
Thank you, I've been meaning to file this bug for weeks.

This is also important for Jetpack, which loads *all* of its scripts (including the SDK) via evalInSandbox.
Comment 4 Michael Wu [:mwu] 2011-06-10 20:24:04 PDT
Created attachment 538661 [details] [diff] [review]
Cache subscripts in startupcache, v2

Updated based on patches in bug 592943
Comment 5 Michael Wu [:mwu] 2011-06-10 20:26:30 PDT
(In reply to comment #3)
> Thank you, I've been meaning to file this bug for weeks.
> 
> This is also important for Jetpack, which loads *all* of its scripts
> (including the SDK) via evalInSandbox.

Oh yeah, jetpack was one of the reasons I worked on this. Are you working on/looking at jetpack startup perf? I can get you a build with caching so you can see how much it helps jetpack if you want to do some measurements.
Comment 6 Dave Townsend [:mossop] 2011-06-10 21:39:21 PDT
Once bug 481603 is fixed I wonder if there might be a bunch of benefits in switching jetpack to use modules where possible to save multiple add-ons using the same version of the SDK having to load the same script multiple times. Would have to be some careful manual refcounting or something to make it useful though I guess
Comment 7 Michael Wu [:mwu] 2011-06-13 20:37:46 PDT
Created attachment 539104 [details] [diff] [review]
Cache subscripts in startupcache, v3

New version which is dependent on the cache invalidation stuff in bug 481603.
Comment 8 Michael Wu [:mwu] 2011-06-14 19:53:37 PDT
Created attachment 539408 [details] [diff] [review]
Cache subscripts in startupcache, v4
Comment 9 Michael Wu [:mwu] 2011-06-29 19:38:38 PDT
Created attachment 543034 [details] [diff] [review]
Cache subscripts in startupcache, v5

Remove some unused includes.
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2011-06-30 18:04:27 PDT
Comment on attachment 543034 [details] [diff] [review]
Cache subscripts in startupcache, v5

Looks good, and with this I think you can remove the static ReadScriptFromStream() and WriteScriptToStream() functions from mozJSComponentLoader.cpp too!

r=jst with that.
Comment 11 Michael Wu [:mwu] 2011-07-09 17:43:04 PDT
Opps. Guess I completely forgot to remove the original function while trying to move it.

Testing on try, will push to m-i once things look ok on try. http://tbpl.mozilla.org/?tree=Try&rev=d36f38e6c687
Comment 13 Mounir Lamouri (:mounir) 2011-07-11 07:22:31 PDT
Merged:
http://hg.mozilla.org/mozilla-central/rev/cb5d55ec3e5d
Comment 14 Kris Maglione [:kmag] 2011-07-12 15:44:03 PDT
What release will this land in? It needs to be documented.
Comment 15 Michael Wu [:mwu] 2011-07-12 16:26:52 PDT
Firefox 8, as noted in target milestone.

This is just an optimization. Developers don't need to do anything about this.
Comment 16 Michael Wu [:mwu] 2011-07-12 16:29:05 PDT
*** Bug 648124 has been marked as a duplicate of this bug. ***
Comment 17 Kris Maglione [:kmag] 2011-07-12 17:00:40 PDT
They do. The various script loading methods are listed on MDC, along with their advantages and disadvantages. The fact this now supports caching can be both an advantage and a disadvantage over other methods, depending on their purposes (especially for developers who rely on getting a fresh load of the script each time it's called).
Comment 18 Eric Shepherd [:sheppy] 2011-07-13 07:09:49 PDT
:kmag updated the docs here:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/mozIJSSubScriptLoader#loadSubScript%28%29

I've added a mention to Firefox 8 for developers.
Comment 19 Alice0775 White 2011-07-15 20:49:35 PDT
(In reply to comment #18)
> :kmag updated the docs here:
> 
> https://developer.mozilla.org/en/XPCOM_Interface_Reference/
> mozIJSSubScriptLoader#loadSubScript%28%29
> 
> I've added a mention to Firefox 8 for developers.

How I can disable caching JS for debugging?

The following settings seems nothing effect.
user_pref("nglayout.debug.disable_xul_fastload", true);
user_pref("nglayout.debug.disable_xul_cache", true);

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