Skip to content
This repository has been archived by the owner on Aug 19, 2021. It is now read-only.

mozilla 8.0, js caching breaks mozrepl #21

Open
retroj opened this issue Oct 3, 2011 · 11 comments
Open

mozilla 8.0, js caching breaks mozrepl #21

retroj opened this issue Oct 3, 2011 · 11 comments

Comments

@retroj
Copy link

retroj commented Oct 3, 2011

From Firefox/XULRunner 8.0, loadSubscript uses the startup cache to speed up repeated loading of js files. This causes a problem for Mozrepl, which uses loadSubscript on a temp file for each bit of code that was entered at the repl. Once it has been called a single time, that file's contents are cached, and all subsequent entries at the repl merely re-evaluate that first, cached entry. See the following bugzilla issue for more information.

https://bugzilla.mozilla.org/show_bug.cgi?id=648125

I had done much research into this problem for a similar situation in the web browser Conkeror, and its "evaluate" command, and the cleanest solution was to invalidate the startup cache before calling loadSubscript on the temp file. The code to do that is as follows:

var obs = Cc["@mozilla.org/observer-service;1"]
.getService(Ci.nsIObserverService);
obs.notifyObservers(null, "startupcache-invalidate", null);

Or if you have imported resource://gre/modules/Services.jsm, then instead of the getService call above, you can just use Services.obs.notifyObservers(...).

The source code of Mozilla's XPIProvider.jsm also has some utils related to this.

@rpl
Copy link
Collaborator

rpl commented Oct 3, 2011

Hi retroj, thanks for reporting this bug.

There's another workaround in my fork:
rpl@28a9b1d

We are evaluating which one has less impact on the xulrunner runtime.

Thanks again,
Luca

@retroj
Copy link
Author

retroj commented Oct 3, 2011

Ah, I see you used the method of altering the name of the temp file for each load. The reason we didn't do this in our situation is that it would lead to growing the startup cache larger and larger over the life of the program. That is something to consider. If you do go that way, I wouldn't use Math.random() because that won't guarantee uniqueness, only randomness, and random failure is the worst kind of failure! :-)

@rpl
Copy link
Collaborator

rpl commented Oct 3, 2011

You're right about the side effects of that workaround and that are the reasons we've not merged it :-(

About cache invalidation: it's possible to do a more selective cleanup? invalidate all the cache content
could generate some other negative side effects?

@retroj
Copy link
Author

retroj commented Oct 3, 2011

Annoyingly, Mozilla doesn't provide a way to invalidate a single item in the startup cache. Even the fact that startup cache invalidation works through an observer rather than there being a complete interface (an nsIStartupCacheManager or whatnot) makes one wonder if this feature was more tacked on than planned. Perhaps this should be brought to Mozilla's attention as a feature request.

The drawback to invalidating the startup cache is the loss of the efficiency it provides with repeat loading of js files. Other than that, there are no side effects. I would choose this method, myself, because although it is a blunt instrument, it is still the right instrument, according to the intent of the design of the startup cache. That design may be flawed, but that's Mozilla's problem.

@bard
Copy link
Owner

bard commented Nov 3, 2011

In cb151c0 and 552ed9b I'm adopting a hybrid approach—cache is flushed when the JavaScript interactor starts, then a new script is cached every time the REPL evaluates something. This should prevent the cache from growing indefinitely.

A more correct behavior would be to flush the cache when the interactor stops, or even better to flush the cache when the browser quits and the REPL is found to have been running. Consider this a quick fix.

I haven't looked much into the cache implementation, but certainly it will have some expiry mechanism? @retroj have you seen anything about that?

@retroj
Copy link
Author

retroj commented Nov 3, 2011

@retroj
Copy link
Author

retroj commented Nov 3, 2011

@retroj
Copy link
Author

retroj commented Nov 3, 2011

I find Cc["@mozilla.org/startupcache/cache;1"] exists but Ci.nsIStartupCache does not. Puzzled. Perhaps it isn't scriptable.

@bard
Copy link
Owner

bard commented Nov 4, 2011

@retroj I was thinking "automatic expiry". I think there ought to be one as otherwise the cache would get bigger and bigger regardless of MozRepl. I'll look into it at some point.

@dmitrii-maksimov
Copy link

can u make one new .xpi pack with this patch?
rpl@28a9b1d
if i trying change repl.jx myself - it was crashed...

@bard
Copy link
Owner

bard commented Nov 15, 2011

Current repo contains a workaround already, it will be in the next xpi.

On Tue, Nov 15, 2011 at 2:08 PM, BlackSnow98
[email protected]
wrote:

can u make one nex .xpi pack with this patch?
rpl@28a9b1d
if i trying change repl.jx myself - it was crashed...


Reply to this email directly or view it on GitHub:
#21 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants