1678458 - Move RefPtr out of ScriptStencil

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Tooru Fujisawa [:arai]]

having RefPtr in ScriptStencil has the following problems:

• RefPtr needs dtor call
• pointer cannot be encoded/decoded directly

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Bug 1678458 - Move RefPtr out of ScriptStencil. r?tcampbell!

Depends on D99652

Comment 2

[Ted Campbell [:tcampbell]]

I wonder what best option for doing this without extra overhead is:

• Parallel arrays, possibly truncated
  • This is the approach I took for SourceExtent. For initial parse the array is same length as scripts, and for delazifications the array is length-0
• HashMap
  • We used this for asmJS. Based on flags, we lookup the asm.js data. This has hash-table lookup, but the asm.js modules are rare.
• Dense array, indexed by script
  • The approach we used for scopes. Adds overhead for the index in each script, even if no data.

I think the general cases we expect are:

• Lazy initial compile: Top-level bytecode and maybe a couple IIFEs ((function() { .. }();) with bytecode
• Full-compile: Common in privileged code. Each script has bytecode.
• Delazification: Script-0 has bytecode, and nothing else does.

It might be possible to do some sort of hybrid where if script-index is > bytecodes-length we then use hashset to cover the first case? I'm not really sure.

Comment 3

[Tooru Fujisawa [:arai]]

I'm trying parallel array + HashMap hybrid way.
there's an issue about when to decide which one to use, without switching between them with copy.

I was about to perform:

• when adding item to the container:
  • if the item is next item of the last parallel array item, add it to parallel array
  • otherwise, add it to HashMap

so that first n-th item with shared data will be stored into parallel array,
and shared data after the first hole will be stored into HashMap.

but this works only if the item is added in the increasing order,
and BytecodeEmitter usage doesn't match, because sharedData is created in post-order while traversing the AST,
and also hoisted functions will screw up the order.

maybe it's better statically decide based on what kind of compilation it is.

Comment 4

[Ted Campbell [:tcampbell]]

(In reply to Tooru Fujisawa [:arai] from comment #3)

maybe it's better statically decide based on what kind of compilation it is.

That probably makes sense. I had also forgotten that BCE runs in a surprising order.

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1678458 - Move RefPtr out of ScriptStencil. r?tcampbell!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/51b93777f63c
Move RefPtr out of ScriptStencil. r=tcampbell

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/51b93777f63c