Bug 133727

Summary:

Structure::get should instantiate DeferGC only when materializing property map

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

JavaScriptCore

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, commit-queue, fpizlo, ggaren, kling, mhahnenberg

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Micro benchmark	none
Avoids creating DeferGC when we can	ggaren: review+, commit-queue: commit-queue-

Ryosuke Niwa

Reported 2014-06-11 01:06:42 PDT

Structure::get is slow in part because DeferGC accesses stuff in vm.heap, which needs to be loaded into memory.

Attachments
Micro benchmark (114.27 KB, text/html) 2014-06-11 01:07 PDT, Ryosuke Niwa	no flags	Details
Avoids creating DeferGC when we can (6.52 KB, patch) 2014-06-11 01:19 PDT, Ryosuke Niwa	ggaren: review+ commit-queue: commit-queue-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Ryosuke Niwa

Comment 1 2014-06-11 01:07:22 PDT

Created attachment 232853 [details] Micro benchmark This benchmark shows 4x regression compared to the shipping Safari.

Ryosuke Niwa

Comment 2 2014-06-11 01:19:57 PDT

Created attachment 232854 [details] Avoids creating DeferGC when we can

Ryosuke Niwa

Comment 3 2014-06-11 01:20:52 PDT

Two samples: 63.4 ms -> 54.7 ms 65.6 ms -> 58.2m ms It consistently show 13-16% improvement on my MacPro but I'm having a hard time getting a stable number over VNC.

Ryosuke Niwa

Comment 4 2014-06-11 01:23:09 PDT

Note that the shipping Safari runs it in 27-28ms.

Ryosuke Niwa

Comment 5 2014-06-11 09:32:12 PDT

Also see https://bugs.webkit.org/show_bug.cgi?id=133739

Geoffrey Garen

Comment 6 2014-06-11 11:28:34 PDT

Comment on attachment 232854 [details] Avoids creating DeferGC when we can r=me

Ryosuke Niwa

Comment 7 2014-06-11 11:33:37 PDT

Comment on attachment 232854 [details] Avoids creating DeferGC when we can Thanks for the review!

WebKit Commit Bot

Comment 8 2014-06-11 12:05:30 PDT

Comment on attachment 232854 [details] Avoids creating DeferGC when we can Rejecting attachment 232854 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 232854, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 169813 = 75d812923619f0ba6dcfa596cd1449df8a934904 r169814 = 3bb9d9dd686ca12b45f1b9e1b01a23c1b128de74 r169815 = 8143a33eb79cf305998fff79a541d4db4c600a8b Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.appspot.com/results/4723952773496832

Ryosuke Niwa

Comment 9 2014-06-11 12:30:49 PDT

Committed r169816: <http://trac.webkit.org/changeset/169816>

Ryosuke Niwa

Comment 10 2014-06-11 13:18:59 PDT

Committed r169822: <http://trac.webkit.org/changeset/169822>

Note You need to log in before you can comment on or make changes to this bug.