Bug 140717 - Change Heap::m_compiledCode to use a Vector
Summary: Change Heap::m_compiledCode to use a Vector
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks: 140774
  Show dependency treegraph
 
Reported: 2015-01-20 22:15 PST by Mark Hahnenberg
Modified: 2015-01-22 11:05 PST (History)
3 users (show)

See Also:


Attachments
Patch (3.93 KB, patch)
2015-01-20 22:22 PST, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (4.21 KB, patch)
2015-01-21 07:32 PST, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
GC logs before (9.70 KB, text/plain)
2015-01-21 11:08 PST, Mark Hahnenberg
no flags Details
GC logs after (9.69 KB, text/plain)
2015-01-21 11:08 PST, Mark Hahnenberg
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2015-01-20 22:15:57 PST
Right now it's a DoublyLinkedList, which is iterated during each collection. This contributes to some of the longish Eden pause times. A Vector would be more appropriate and would also allow ExecutableBase to be 2 pointers smaller.
Comment 1 Mark Hahnenberg 2015-01-20 22:22:47 PST
Created attachment 245049 [details]
Patch
Comment 2 Benjamin Poulain 2015-01-21 01:02:06 PST
Hi Mark :)
Comment 3 Mark Hahnenberg 2015-01-21 07:32:23 PST
Created attachment 245062 [details]
Patch
Comment 4 Mark Hahnenberg 2015-01-21 07:33:10 PST
(In reply to comment #2)
> Hi Mark :)

Hello :-)
Comment 5 Andreas Kling 2015-01-21 09:57:26 PST
Comment on attachment 245062 [details]
Patch

r=me

Good job! *claps*
Comment 6 Mark Hahnenberg 2015-01-21 11:08:36 PST
Created attachment 245070 [details]
GC logs before
Comment 7 Mark Hahnenberg 2015-01-21 11:08:52 PST
Created attachment 245071 [details]
GC logs after
Comment 8 Mark Hahnenberg 2015-01-21 11:11:32 PST
Just as a data point, I've attached some GC timing logs gathered by running the Octane 2.0 benchmark in browser before and after this change.

ClearUnmarkedExecutables goes from:

[34505] ClearUnmarkedExecutables (All): 133.93ms (avg. 0.25, min. 0.00, max. 5.35, count 544)
[34505] ClearUnmarkedExecutables (Eden): 121.88ms (avg. 0.24, min. 0.03, max. 5.35, count 506)
[34505] ClearUnmarkedExecutables (Full): 12.05ms (avg. 0.32, min. 0.00, max. 3.55, count 38)

to:

[34530] ClearUnmarkedExecutables (All): 14.53ms (avg. 0.03, min. 0.00, max. 1.76, count 556)
[34530] ClearUnmarkedExecutables (Eden): 13.22ms (avg. 0.03, min. 0.01, max. 1.76, count 518)
[34530] ClearUnmarkedExecutables (Full): 1.30ms (avg. 0.03, min. 0.00, max. 0.66, count 38)

overall time goes down as well. Before:

[34505] Collect (All): 1499.15ms (avg. 2.76, min. 0.43, max. 42.51, count 544)
[34505] Collect (Eden): 931.76ms (avg. 1.84, min. 0.51, max. 42.51, count 506)
[34505] Collect (Full): 567.39ms (avg. 14.93, min. 0.43, max. 31.69, count 38)

after:

[34530] Collect (All): 1372.41ms (avg. 2.47, min. 0.25, max. 37.83, count 556)
[34530] Collect (Eden): 813.91ms (avg. 1.57, min. 0.45, max. 37.83, count 518)
[34530] Collect (Full): 558.51ms (avg. 14.70, min. 0.25, max. 25.97, count 38)

(despite doing more collections :-)
Comment 9 WebKit Commit Bot 2015-01-21 18:56:23 PST
Comment on attachment 245062 [details]
Patch

Clearing flags on attachment: 245062

Committed r178884: <http://trac.webkit.org/changeset/178884>
Comment 10 WebKit Commit Bot 2015-01-21 18:56:26 PST
All reviewed patches have been landed.  Closing bug.