Bug 144476

Summary:

REGRESSION(r183570): jslib-traverse-jquery is 22% slower

Product:

WebKit

Reporter:

Geoffrey Garen <ggaren>

Component:

New Bugs

Assignee:

Geoffrey Garen <ggaren>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, buildbot, kling, mark.lam, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Archive of layout-test-results from ews104 for mac-mavericks-wk2	none
Archive of layout-test-results from ews102 for mac-mavericks	none
Patch	none
Patch	sam: review+

Geoffrey Garen

Reported 2015-04-30 15:50:38 PDT

REGRESSION(r183570): jslib-traverse-jquery is 22% slower

Attachments
Patch (2.33 KB, patch) 2015-04-30 16:01 PDT, Geoffrey Garen	no flags	Details Formatted Diff Diff
Patch (2.33 KB, patch) 2015-04-30 16:36 PDT, Geoffrey Garen	no flags	Details Formatted Diff Diff
Patch (3.30 KB, patch) 2015-05-01 12:00 PDT, Geoffrey Garen	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (1.08 MB, application/zip) 2015-05-01 12:25 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews102 for mac-mavericks (862.03 KB, application/zip) 2015-05-01 12:33 PDT, Build Bot	no flags	Details
Patch (1.35 KB, patch) 2015-05-01 16:41 PDT, Geoffrey Garen	no flags	Details Formatted Diff Diff
Patch (3.64 KB, patch) 2015-05-02 15:09 PDT, Geoffrey Garen	sam: review+	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Geoffrey Garen

Comment 1 2015-04-30 16:01:33 PDT

Created attachment 252100 [details] Patch

Geoffrey Garen

Comment 2 2015-04-30 16:13:46 PDT

Comment on attachment 252100 [details] Patch Looks like I broke everything :(.

Geoffrey Garen

Comment 3 2015-04-30 16:36:16 PDT

Created attachment 252112 [details] Patch

Mark Lam

Comment 4 2015-04-30 17:07:01 PDT

Comment on attachment 252112 [details] Patch r=me too.

Andreas Kling

Comment 5 2015-04-30 18:13:01 PDT

Comment on attachment 252112 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252112&action=review > Source/JavaScriptCore/ChangeLog:13 > + (b) the nodes are usually already sorted. I think they will always be sorted, originating from querySelector APIs which always return nodes in document order. :) > Source/JavaScriptCore/ChangeLog:22 > + 82% speedup on jslib-traverse-jquery, bringing us to 40% faster than > + the unregressed baseline. Kickass!

Geoffrey Garen

Comment 6 2015-05-01 11:43:40 PDT

Comment on attachment 252112 [details] Patch Still broken :(. BTW, the nodes in the worst tests are never in order or reverse order. I don't know why yet.

Geoffrey Garen

Comment 7 2015-05-01 12:00:34 PDT

Created attachment 252162 [details] Patch

Build Bot

Comment 8 2015-05-01 12:25:12 PDT

Comment on attachment 252162 [details] Patch Attachment 252162 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5776924949348352 New failing tests: fast/dom/navigator-detached-no-crash.html js/sort-large-array.html transitions/transition-end-event-multiple-03.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.1_T3.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.2_T1.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.2_T3.html fast/xmlhttprequest/xmlhttprequest-get.xhtml sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.1_T1.html http/tests/workers/worker-importScriptsOnError.html js/dom/regexp-caching.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A3_T2.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A3_T1.html js/reserved-words-strict.html fast/dom/plugin-attributes-enumeration.html js/dom/global-constructors-attributes-dedicated-worker.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.1_T2.html fast/dom/event-handler-attributes.html js/comparefn-sort-stability.html js/dom/global-constructors-attributes.html sputnik/Conformance/13_Function_Definition/13.2_Creating_Function_Objects/S13.2.1_A5_T1.html jquery/traversing.html js/Object-getOwnPropertyNames.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.2_T2.html

Build Bot

Comment 9 2015-05-01 12:25:15 PDT

Created attachment 252165 [details] Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5

Build Bot

Comment 10 2015-05-01 12:33:18 PDT

Comment on attachment 252162 [details] Patch Attachment 252162 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6060547409707008 New failing tests: fast/dom/navigator-detached-no-crash.html js/sort-large-array.html transitions/transition-end-event-multiple-03.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.1_T3.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.2_T1.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.2_T3.html fast/xmlhttprequest/xmlhttprequest-get.xhtml sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.1_T1.html http/tests/workers/worker-importScriptsOnError.html js/dom/regexp-caching.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A3_T2.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A3_T1.html js/reserved-words-strict.html fast/dom/plugin-attributes-enumeration.html js/dom/global-constructors-attributes-dedicated-worker.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.1_T2.html fast/dom/event-handler-attributes.html js/comparefn-sort-stability.html js/dom/global-constructors-attributes.html sputnik/Conformance/13_Function_Definition/13.2_Creating_Function_Objects/S13.2.1_A5_T1.html jquery/traversing.html js/Object-getOwnPropertyNames.html sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A2.2_T2.html

Build Bot

Comment 11 2015-05-01 12:33:21 PDT

Created attachment 252166 [details] Archive of layout-test-results from ews102 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-mavericks Platform: Mac OS X 10.9.5

Geoffrey Garen

Comment 12 2015-05-01 16:41:18 PDT

Created attachment 252195 [details] Patch

Geoffrey Garen

Comment 13 2015-05-02 15:09:21 PDT

Created attachment 252250 [details] Patch

Andreas Kling

Comment 14 2015-05-02 17:02:38 PDT

Comment on attachment 252250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252250&action=review > Source/JavaScriptCore/ChangeLog:29 > + The solution is simple enough: Call compareDocumentPosition(b, a) instead. Amazing. r=me too. > Source/JavaScriptCore/ChangeLog:48 > + Another option is for elements to keep a compareDocumentPosition cache, > + like a node list cache, which allows you to determine the absolute > + position of a node using a hash lookup. I will leave this as an exercise > + for kling. Or we can just wait for anttik to finish his legendary CoreNode refactoring where container nodes have arrays of children, and compareDocumentPosition can be implemented using simple pointer comparison for siblings. :]

Geoffrey Garen

Comment 15 2015-05-04 10:28:51 PDT

Committed r183749: <http://trac.webkit.org/changeset/183749>

Simon Fraser (smfr)

Comment 16 2015-05-04 11:25:51 PDT

Comment on attachment 252250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252250&action=review > Source/JavaScriptCore/ChangeLog:40 > + A fully principled soultion to this problem would probably do something soultion

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