Add the source text back to the script struct

[jungkees]

Service workers reference the script's source text to compare two service workers' version. The script's source text has been removed from HTML at some point. Please add it back. We plan to use it for both the classic script comparison and the module script comparison.

Related issue: w3c/ServiceWorker#1212.

/cc @domenic

[domenic]

Thanks for filing over here; this kept slipping off the top of my mind. I'll try to get to it soon!!

For my personal curiousity, I'd love to hear from implementers how they plan to implement this, and maybe add a note to the spec about it. Because keeping the script source in memory all the time seems like a lot of overhead. Would implementers do that only for service worker scripts? Or would they page them out to disk, perhaps? /cc @whatwg/modules @wanderview

[jungkees]

Chromium stores the actual script resources in the disk cache and maintains a separate in-memory map that only has url to resource record info (id and such that's tied to the actual resource.) I believe Firefox, Edge, and Safari have similar implementations already. Adding a note to the spec seems like a good idea. /cc @mattto @aliams @cdumez

[jungkees]

To clarify, Chromium doesn't support module scripts for service workers yet.

[annevk]

(It seems implementations could store a SHA-2 hash as well and use that for comparison against a new script.)

[jungkees]

Chromium's comparing the existing script progressively with the new script as it is read from network so when it detects a byte difference it starts writing the new one to the disk right away.

I think using hash could be one of the ways to implement this too. It seems like implementation options.

[wanderview]

We currently keep the script source in memory while performing a SW update. We haven't really implemented any early abort optimizations, etc.

I'm not sure what we do for script sources in spidermonkey currently.

[aliams]

We use a hash for comparison which is generated after we retrieve the new script file.

[nyaxt]

@hiroshige-g @otherdaniel who was poking at the Blink code around the area.
I believe we (Blink+V8) keep the script source text in memory, even after execution, so that we can answer Function.prototype.toString().

One question: Is this about decoded source text, or raw undecoded bytestream? Keeping raw undecoded bytestream around can come with additional overhead.

[annevk]

What does decoded source text mean? Is that only text decoding or something more? Note that if Edge computes the hash based on the byte stream they'll get different results, so we should actually make this very clear (and add tests for the difference).

[otherdaniel]

@nyaxt: For V8+Blink:

• V8 keeps a reference to all source text as v8::String.
• The exact shape of that string depends on how the source was handed down from Blink. Usually, it's External(One|Two)ByteString, meaning it wraps a piece of memory owned by Blink. ServiceWorkers might handle this differently; not sure about that.
• The script string is kept until the GC no longer finds any direct or indirect references.
  • (indirect refs include: the script object; any compiled function; any substring that references the original; etc.)
• The raw bytestream is only temporarily used (while streaming), and we won't keep a reference past the initial compile (which occurs when streaming is finished).

@annevk:

• decoded == ASCII or UCS-2, derived from whichever text encoding the resource originally was in.

[annevk]

Okay, so different byte sequences that decode to the same code points end up not counting as a different resource in Chrome. This could be due to a leading BOM being stripped, a different encoding being used, and different byte sequences decoding to the same code point (most common with errors becoming U+FFFD).

[hiroshige-g]

Looks fine.

According to @jungkees's comments and offline discussion with @horo-t, I expect V8+Blink is not affected because the update algorithm referenced in w3c/ServiceWorker#1212 is implemented in the browser process (which is quite far from Blink's #concept-script impl blink::Script).

Even when we add source text to blink::Script, I expect it doesn't cause memory regression (+1 to @nyaxt @otherdaniel comments).
But I think it's better not to add source text to blink::Script for implementation flexibility, and thus it might be better to limit the source text's usage (e.g. by add a note "comparison only").

[domenic]

So in terms of actually working on this, do we want to store the response bytes, or the source text? If we store source text, then e.g. two scripts should compare the same even if their bytes are different, per all the factors @annevk lists.

Based on what I'm hearing so far in this thread:

• Edge: unclear; @aliams is the hash being done on the source bytes or the decoded/BOM-stripped source text?
• Gecko: @wanderview said "script source" implying to me source text, not bytes
• Blink: source text
• WebKit: ?

I hope someone can help write tests for these cases.