Bug 13385 - [js-collector-tweaks] Shrink List (and therefore ActivationImp), discard arguments List when no longer needed
Summary: [js-collector-tweaks] Shrink List (and therefore ActivationImp), discard argu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Maciej Stachowiak
URL:
Keywords:
Depends on:
Blocks: 13389
  Show dependency treegraph
 
Reported: 2007-04-18 00:12 PDT by Maciej Stachowiak
Modified: 2007-04-22 21:36 PDT (History)
0 users

See Also:


Attachments
05-js-gc-arguments-discard+list-shrink.patch.txt (7.27 KB, patch)
2007-04-18 00:12 PDT, Maciej Stachowiak
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 2007-04-18 00:12:12 PDT
Discard the arguments List for an ActivationImp when the corresponding Context is destroyed (1.7% speedup, and this will enable further optimizations).
Comment 1 Maciej Stachowiak 2007-04-18 00:12:43 PDT
Created attachment 14066 [details]
05-js-gc-arguments-discard+list-shrink.patch.txt
Comment 2 Darin Adler 2007-04-18 11:15:10 PDT
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
Comment 3 Maciej Stachowiak 2007-04-18 15:01:27 PDT
(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.