145412 – REGRESSION: These sorting idioms used by Peacekeeper and Browsermark are ~20X slower

Bug 145412 - REGRESSION: These sorting idioms used by Peacekeeper and Browsermark are ~20X slower

Summary: REGRESSION: These sorting idioms used by Peacekeeper and Browsermark are ~20X...

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	New Bugs (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Geoffrey Garen

URL:
Keywords:

Depends on:
Blocks:

Reported:	2015-05-27 11:33 PDT by Geoffrey Garen
Modified:	2015-06-01 11:40 PDT (History)
CC List:	2 users (show)

See Also:

Attachments
Patch (1.39 KB, patch) 2015-05-27 11:50 PDT, Geoffrey Garen	no flags	Details \| Formatted Diff \| Diff
Patch (4.11 KB, patch) 2015-05-27 15:54 PDT, Geoffrey Garen	no flags	Details \| Formatted Diff \| Diff
Patch (4.23 KB, patch) 2015-05-27 16:26 PDT, Geoffrey Garen	no flags	Details \| Formatted Diff \| Diff
Patch (4.18 KB, patch) 2015-05-29 17:11 PDT, Geoffrey Garen	no flags	Details \| Formatted Diff \| Diff
Patch (4.18 KB, patch) 2015-05-29 17:13 PDT, Geoffrey Garen	darin: review+	Details \| Formatted Diff \| Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

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

Description Geoffrey Garen 2015-05-27 11:33:19 PDT

REGRESSION: These sorting idioms used by Peacekeeper and Browsermark are ~20X slower

Comment 1 Geoffrey Garen 2015-05-27 11:50:40 PDT

Created attachment 253797 [details]
Patch

Comment 2 Geoffrey Garen 2015-05-27 11:51:12 PDT

This is a partial fix for the regression.

Comment 3 Geoffrey Garen 2015-05-27 11:53:28 PDT

(I'm still waiting on a build to get performance numbers.)

Comment 4 Darin Adler 2015-05-27 11:53:59 PDT

Comment on attachment 253797 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=253797&action=review

> Source/JavaScriptCore/ChangeLog:10
> +        Use @toString instead of the String constructor because calls to the
> +        String constructor are never optimized. (See
> +        https://bugs.webkit.org/show_bug.cgi?id=144458.)

What’s the downside of using @toString? Would we want to change this back some day if resolved bug 144458?

Comment 5 Geoffrey Garen 2015-05-27 12:59:45 PDT

There's no downside to @toString.

Assuming that ES6 settles on making ToString and String identical in behavior, we might eventually remove @toString once @String is fast.

Comment 6 Geoffrey Garen 2015-05-27 13:42:14 PDT

Committed r184917: <http://trac.webkit.org/changeset/184917>

Comment 7 Geoffrey Garen 2015-05-27 15:54:06 PDT

Reopening to attach new patch.

Comment 8 Geoffrey Garen 2015-05-27 15:54:07 PDT

Created attachment 253806 [details]
Patch

Comment 9 Geoffrey Garen 2015-05-27 16:26:39 PDT

Created attachment 253811 [details]
Patch

Comment 10 Geoffrey Garen 2015-05-27 16:43:35 PDT

Committed r184926: <http://trac.webkit.org/changeset/184926>

Comment 11 Geoffrey Garen 2015-05-29 17:11:10 PDT

Reopening to attach new patch.

Comment 12 Geoffrey Garen 2015-05-29 17:11:12 PDT

Created attachment 253940 [details]
Patch

Comment 13 Geoffrey Garen 2015-05-29 17:13:54 PDT

Created attachment 253941 [details]
Patch

Comment 14 Geoffrey Garen 2015-06-01 11:40:49 PDT

Committed r185067: <http://trac.webkit.org/changeset/185067>