750185 - Investigate which script caches are still useful after compartment-per-global

Status: NEW

(Core :: XUL, defect)

Description

[Bobby Holley (:bholley)]

Right now, we cache compiled chrome scripts in places like the XUL prototype cache and run them against multiple globals. With compartment-per-global, this doesn't work, because the script is in the wrong compartment. So in bug 743034, luke just added the simple hack to clone the script if it comes from a different compartment.

There's no point in caching this clone, since it's a per-global/per-compartment thing. But this begs the question of whether we should be holding onto these cached scripts at all. I have no idea how much memory they consume, but if they're unnecessary we should probably look into it.

Comment 1

[Boris Zbarsky [:bzbarsky]]

> There's no point in caching this clone,

It avoids having to read in all the data off disk as XDR (or worse yet having to bytecode-compile the script), right?

But yes, it would be good to get updated performance measurements here.

Comment 2

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky (:bz) from comment #1)
> > There's no point in caching this clone,
> 
> It avoids having to read in all the data off disk as XDR (or worse yet
> having to bytecode-compile the script), right?

We already XDR on clone, no? Not sure whether we reuse the bytecode.

My point was that the clones themselves don't need to be cached: We need a separate clone per compartment, and there's only one global in a compartment, so we can't reuse the script. Unless there are cases when we re-execute the same script against a global multiple times?

Comment 3

[Boris Zbarsky [:bzbarsky]]

> We already XDR on clone, no?

I have no idea.  Do we?

Again, the point of the script cache is that opening a new window should not need to read scripts off disk and _definitely_ should not need to perfom bytecode compilation.

> My point was that the clones themselves don't need to be cached

They aren't, right?  There's just a single prototype instance cached, I'd think.

Comment 4

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky (:bz) from comment #3)
> > We already XDR on clone, no?
> 
> I have no idea.  Do we?

http://mxr.mozilla.org/mozilla-central/source/js/src/jsscript.cpp#1739

There's no file I/O, though.

> Again, the point of the script cache is that opening a new window should not
> need to read scripts off disk and _definitely_ should not need to perfom
> bytecode compilation.

That makes sense.

> 
> > My point was that the clones themselves don't need to be cached
> 
> They aren't, right?  There's just a single prototype instance cached, I'd
> think.

Yes. But in comment 1 you implied that there would be a benefit to caching the clone.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Oh, sorry.  Didn't mean to; I thought you were talking about caching the original script, not the clone.

Comment 6

[Luke Wagner [:luke]]

Also, if we do bug 679940 (which allows bug 679942), even less will need to be cloned for cached scripts.

Comment 7

[Justin Lebar (not reading bugmail)]

Can you guys give us (MemShrink) an idea of how much memory is at play here?

Comment 8

[Bobby Holley (:bholley)]

I don't know, and jst didn't know :-(

Though from what Boris said in this bug, it sounds like we still want some sort of cache (to avoid the I/O). Maybe it's not memshrink-worthy? Or maybe boris could say more.

Comment 9

[Boris Zbarsky [:bzbarsky]]

I have no idea how much memory is involved exactly, but it's bounded above by the memory about:memory lists for scripts in the relevant compartment(s), right?

Of course if we have a way to just ask a JSScript for how much memory it uses we could just enumerate the cache an dump out a number.