Bug 13385 – [js-collector-tweaks] Shrink List (and therefore ActivationImp), discard arguments List when no longer needed

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Maciej Stachowiak]

Discard the arguments List for an ActivationImp when the corresponding Context is destroyed (1.7% speedup, and this will enable further optimizations).

[Maciej Stachowiak]

Created attachment 14066 [details]
05-js-gc-arguments-discard+list-shrink.patch.txt

[Darin Adler]

Comment on attachment 14066 [details]
05-js-gc-arguments-discard+list-shrink.patch.txt

+    if (activation)
+      activation->_arguments.reset();

This is 2-space indented in a 4-space-indented function.

+    friend class Context;

I'd really prefer to see this done with a public function instead of making Context a friend.

I think we can make additional simplifications to List if we don't need the !_needsMarking version.

r=me

[Maciej Stachowiak]

(In reply to comment #2)
> (From update of attachment 14066 [details] [edit])
> +    if (activation)
> +      activation->_arguments.reset();
> 
> This is 2-space indented in a 4-space-indented function.
> 
> +    friend class Context;
> 
> I'd really prefer to see this done with a public function instead of making
> Context a friend.

I'll fix that.

> I think we can make additional simplifications to List if we don't need the
> !_needsMarking version. 

It's actually the _needsMarking version that was removed - now no Lists need explicit marking. I'm not sure if there are other easy simplifications to be made.