665167 - [jsdbg2] Eliminate non-held scripts

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

I think eval scripts could be stored in the heldScripts WeakMap without any problems.

Comment 1

[Jason Orendorff [:jorendorff]]

Slightly changing the meaning of this bug. Patches coming.

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: part 1 - Give eval scripts a Script object

This eliminates TCF_NEED_SCRIPT_OBJECT; all non-function scripts need a script object now.

JSScript::u.nextToGC is no longer an accurate name, so rename it to JSScript::evalCacheChain. It can no longer be in a union with JSScript::u.object, since JSScript::object is now populated for eval scripts; so make them separate fields.

Eliminate code to eagerly destroy scripts outside of GC.

-65 lines of code net. Tests pass.

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: part 2 - Give function scripts a pointer to the Function object that keeps them gc-live

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: part 3 - Remove Debugger.Script.prototype.live

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: part 4 - Remove non-held script support from js::Debugger

Comment 6

[Jason Orendorff [:jorendorff]]

Part 4 is -160 lines net.

Changing things so that this js::Debugger method can exist, with this signature:
  JSObject *wrapScript(JSContext *cx, JSScript *script);
is the point of this patch queue. That makes getAllScripts (bug 676281) possible.

Comment 7

[Jason Orendorff [:jorendorff]]

Attachment: part 5 - Make wrapScript more like getScriptFrame

As soon as I wrote that, I realized that the signature could be improved a little. The callers all want the result in a Value.

Comment 8

[Jason Orendorff [:jorendorff]]

Attachment: part 5 - Make wrapScript more like getScriptFrame

Comment 9

[Jason Orendorff [:jorendorff]]

Attachment: part 2 - Populate JSScript::object for function scripts

Whoops, found two more spots where function scripts are created.
The total number is 4:
  - JSScript::NewScriptFromCG, compilation
  - js_XDRFunctionObject, deserialization
  - Function.prototype is custom-built in js_InitFunctionClass
  - js_CloneFunctionObject

Comment 10

[Jason Orendorff [:jorendorff]]

Attachment: part 2 - Populate JSScript::object for function scripts

Fix a bug.

Comment 11

[Igor Bukanov]

Jason - can you test the patches against TDHTML on try server? I suspect that they would show the same regression as I observe in bug 674251. For the try run I use:

try: -b o  -p all -u none -t nochrome

Comment 12

[Jim Blandy :jimb]

Do you still need a review on this, or has it been made obsolete by JSScript becoming an expressible value.

Comment 13

[Jim Blandy :jimb]

I had no intention of changing the Version and Target Milestone in the previous comment; I don't know what happened.

Comment 14

[Igor Bukanov]

(In reply to Jim Blandy :jimb from comment #12)
> an expressible value.

What is "an expressible value"?

Comment 15

[Jim Blandy :jimb]

The current sources have only a single table of scripts, managing both eval scripts and other sorts of scripts, so this bug is resolved.