Speed up updates of existing CSS properties from JS.
Created attachment 133868 [details] Patch
Comment on attachment 133868 [details] Patch Attachment 133868 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12134998 New failing tests: fast/events/ondrop-text-html.html inspector/styles/styles-update-from-js.html inspector/styles/styles-new-API.html editing/pasteboard/onpaste-text-html.html fast/mutation/observe-attributes.html editing/pasteboard/data-transfer-items.html fast/dom/css-set-property-exception.html
Created attachment 133877 [details] Archive of layout-test-results from ec2-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
(In reply to comment #2) > (From update of attachment 133868 [details]) > Attachment 133868 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/12134998 > > New failing tests: > fast/events/ondrop-text-html.html > inspector/styles/styles-update-from-js.html > inspector/styles/styles-new-API.html > editing/pasteboard/onpaste-text-html.html > fast/mutation/observe-attributes.html > editing/pasteboard/data-transfer-items.html > fast/dom/css-set-property-exception.html All these failure are because the updated properties are not added at the end of the list of properties therefore the cssText is different. You may say I broke existing behavior but then I ran fast/dom/css-set-property-exception.html in both Firefox 11 and Opera. Firefox 11 returns the same order as I return with the patch (meaning that the updated CSS property is not at the end of the list but I update the current value) Opera returns the current behavior, the updated property (e.g. display) is at the end. Therefore it's not safe to rely on that order. Before I update or not the expected output any opinion?
> Before I update or not the expected output any opinion? I think it is ok. If the other browser do not enforce any specific order, we do not break any compatibility.
Created attachment 133908 [details] Patch
Modified version of CSSPropertySetterGetter PerfTestRunner.run(function() { for (key in properties) { var value = div.style[key]; //div.style[key] = ""; div.style[key] = properties[key]; } }, 5000); Without the patch : darktears@DarthVader:~/dev/troll/webkit$ WEBKITOUTPUTDIR=/home/darktears/dev/troll/webkit2-trunk run-perf-tests --platform=qt PerformanceTests/CSS/CSSPropertySetterGetter.html Running CSS/CSSPropertySetterGetter.html (1 of 1) RESULT CSS: CSSPropertySetterGetter= 940.65 ms median= 940.5 ms, stdev= 1.15217186218 ms, min= 939.0 ms, max= 943.0 ms Finished: 20.161777 s WEBKITOUTPUTDIR=/home/darktears/dev/troll/webkit2-trunk run-perf-tests 20.16s user 0.06s system 99% cpu 20.408 total With the patch : darktears@DarthVader:~/dev/troll/webkit$ WEBKITOUTPUTDIR=/home/darktears/dev/troll/webkit2-trunk run-perf-tests --platform=qt PerformanceTests/CSS/CSSPropertySetterGetter.html Running CSS/CSSPropertySetterGetter.html (1 of 1) RESULT CSS: CSSPropertySetterGetter= 689.75 ms median= 689.0 ms, stdev= 2.11837201643 ms, min= 688.0 ms, max= 697.0 ms Finished: 14.896826 s WEBKITOUTPUTDIR=/home/darktears/dev/troll/webkit2-trunk run-perf-tests 14.92s user 0.06s system 98% cpu 15.163 total
Comment on attachment 133908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133908&action=review Great idea. r=me > Source/WebCore/ChangeLog:10 > + Improve the way we handle updating an existing CSS property by replacing its value > + by the new one rather than removing the old value and then adding the new one. This > + speed up the update of existing properties by 28%. Is this a 28% improvement on a test we have in PerformanceTests/? If so, you should mention it by name. :)
(In reply to comment #8) > (From update of attachment 133908 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133908&action=review > > Great idea. r=me > > > Source/WebCore/ChangeLog:10 > > + Improve the way we handle updating an existing CSS property by replacing its value > > + by the new one rather than removing the old value and then adding the new one. This > > + speed up the update of existing properties by 28%. > > Is this a 28% improvement on a test we have in PerformanceTests/? If so, you should mention it by name. :) I have added https://bugs.webkit.org/show_bug.cgi?id=82321 which covers the performance improvement.
Created attachment 134268 [details] Patch for landing
Comment on attachment 134268 [details] Patch for landing Clearing flags on attachment: 134268 Committed r112387: <http://trac.webkit.org/changeset/112387>
All reviewed patches have been landed. Closing bug.