Bug 144476 - REGRESSION(r183570): jslib-traverse-jquery is 22% slower
Summary: REGRESSION(r183570): jslib-traverse-jquery is 22% slower
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-30 15:50 PDT by Geoffrey Garen
Modified: 2015-05-04 11:25 PDT (History)
5 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2015-04-30 15:50:38 PDT
REGRESSION(r183570): jslib-traverse-jquery is 22% slower
Comment 1 Geoffrey Garen 2015-04-30 16:01:33 PDT
Created attachment 252100 [details]
Patch
Comment 2 Geoffrey Garen 2015-04-30 16:13:46 PDT
Comment on attachment 252100 [details]
Patch

Looks like I broke everything :(.
Comment 3 Geoffrey Garen 2015-04-30 16:36:16 PDT
Created attachment 252112 [details]
Patch
Comment 4 Mark Lam 2015-04-30 17:07:01 PDT
Comment on attachment 252112 [details]
Patch

r=me too.
Comment 5 Andreas Kling 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!
Comment 6 Geoffrey Garen 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.
Comment 7 Geoffrey Garen 2015-05-01 12:00:34 PDT
Created attachment 252162 [details]
Patch
Comment 8 Build Bot 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
Comment 9 Build Bot 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
Comment 10 Build Bot 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
Comment 11 Build Bot 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
Comment 12 Geoffrey Garen 2015-05-01 16:41:18 PDT
Created attachment 252195 [details]
Patch
Comment 13 Geoffrey Garen 2015-05-02 15:09:21 PDT
Created attachment 252250 [details]
Patch
Comment 14 Andreas Kling 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. :]
Comment 15 Geoffrey Garen 2015-05-04 10:28:51 PDT
Committed r183749: <http://trac.webkit.org/changeset/183749>
Comment 16 Simon Fraser (smfr) 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