903212 - Remove nsIScriptContext::ScriptEvaluated

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Bobby Holley (:bholley)]

Once bug 901364 lands and the slow script dialog machinery is no longer tangled up in nsJSContext, ScriptEvaluted can more or less go away, which is awesome. It does a few more vestigial things, but those can be handled relatively easily, I think.

Comment 1

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=b3f30d476770

Comment 2

[Bobby Holley (:bholley)]

Assuming that try-opt builds and tinderbox m-c builds are comparable, this does not affect sunspider times:

Baseline: http://bit.ly/13XtFwT
Patches: http://bit.ly/1czFmQ6

Comment 3

[Bobby Holley (:bholley)]

Attachment: Part 1 - Remove activity tracking from nsJSContext. v1

This only has an impact when we enable explict compartmental GCs, which have
been preffed off for a while.

Comment 4

[Bobby Holley (:bholley)]

Attachment: Part 2 - Add a GC when popping a script entry point. v1

I think that this is more or less isomorphic with the MaybeGC we do in
ScriptEvaluated in the cases that matter. And doing it in a spec-defined
place is helpful in getting rid of spec-nonsensical machinery.

Comment 5

[Bobby Holley (:bholley)]

Attachment: Part 3 - Remove ScriptEvaluated, and simplify callers. v1

Comment 6

[Bobby Holley (:bholley)]

Attachment: Part 4 - Remove nsIXPCScriptNotify machinery. v1

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 790005 [details] [diff] [review]
Part 1 - Remove activity tracking from nsJSContext. v1

You should remove sDisableExplicitCompartmentGC everywhere, and the
pref controlling it from all.js (the pref has been true over a year)
javascript.options.mem.disable_explicit_compartment_gc
And remove NS_MAX_COMPARTMENT_GC_COUNT too.
And sCompartmentGCCount
And I think sContextList too.

Comment 8

[Bobby Holley (:bholley)]

Comment on attachment 790005 [details] [diff] [review]
Part 1 - Remove activity tracking from nsJSContext. v1

(In reply to Olli Pettay [:smaug] from comment #7)
> You should remove sDisableExplicitCompartmentGC everywhere, and the
> pref controlling it from all.js (the pref has been true over a year)
> javascript.options.mem.disable_explicit_compartment_gc
> And remove NS_MAX_COMPARTMENT_GC_COUNT too.
> And sCompartmentGCCount
> And I think sContextList too.

Can I do that in a followup patch please?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: part 1, remove pref etc

I'd prefer not leaving trivial stuff to followups.
Here is the patch which removes the pref and various variables.

Comment 10

[Bobby Holley (:bholley)]

Attachment: Part 5 - Remove explicit compartment GC machinery. v1

I need the context list stuff for my patches in bug 899367.

Comment 11

[Bobby Holley (:bholley)]

Green try push in comment 1, but I added an extra patch that smaug asked for, so doing a quick linux64 try push just to be safe:

https://tbpl.mozilla.org/?tree=Try&rev=f638d36d5a74

Comment 12

[Bobby Holley (:bholley)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/82f8c3925056
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/76cb6754a52e
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/355f9a21c432
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/8de0f0b74488
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/0b3cd7829be9

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/82f8c3925056
https://hg.mozilla.org/mozilla-central/rev/76cb6754a52e
https://hg.mozilla.org/mozilla-central/rev/355f9a21c432
https://hg.mozilla.org/mozilla-central/rev/8de0f0b74488
https://hg.mozilla.org/mozilla-central/rev/0b3cd7829be9