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

NEW
Unassigned

Status

()

Core
XUL
6 years ago
6 years ago

People

(Reporter: bholley, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

(Reporter)

Description

6 years ago
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.
> 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.
Component: DOM → XUL
QA Contact: general → xptoolkit.widgets
(Reporter)

Comment 2

6 years ago
(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?
> 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.
(Reporter)

Comment 4

6 years ago
(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.
Oh, sorry.  Didn't mean to; I thought you were talking about caching the original script, not the clone.

Comment 6

6 years ago
Also, if we do bug 679940 (which allows bug 679942), even less will need to be cloned for cached scripts.
Can you guys give us (MemShrink) an idea of how much memory is at play here?
(Reporter)

Comment 8

6 years ago
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.
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.
Whiteboard: [MemShrink] → [MemShrink:P2]
You need to log in before you can comment on or make changes to this bug.