RESOLVED FIXED 61038
Web Inspector: Serious performance regression during continuous focused element style updates
https://bugs.webkit.org/show_bug.cgi?id=61038
Summary Web Inspector: Serious performance regression during continuous focused eleme...
Alexander Pavlov (apavlov)
Reported 2011-05-18 04:24:38 PDT
Attachments
[PATCH] Suggested solution (3.87 KB, patch)
2011-05-18 07:41 PDT, Alexander Pavlov (apavlov)
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-02 (1.15 MB, application/zip)
2011-05-18 11:29 PDT, WebKit Review Bot
no flags
[PATCH] A different approach applied (20.60 KB, patch)
2011-05-19 09:48 PDT, Alexander Pavlov (apavlov)
webkit.review.bot: commit-queue-
[PATCH] Offline comments addressed (22.18 KB, patch)
2011-05-20 06:56 PDT, Alexander Pavlov (apavlov)
no flags
[PATCH] Rebase (20.03 KB, patch)
2011-06-09 03:26 PDT, Alexander Pavlov (apavlov)
pfeldman: review-
[PATCH] Comments addressed (21.06 KB, patch)
2011-06-15 08:56 PDT, Alexander Pavlov (apavlov)
pfeldman: review+
Alexander Pavlov (apavlov)
Comment 1 2011-05-18 07:41:30 PDT
Created attachment 93912 [details] [PATCH] Suggested solution
WebKit Review Bot
Comment 2 2011-05-18 11:29:27 PDT
Comment on attachment 93912 [details] [PATCH] Suggested solution Attachment 93912 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8708641 New failing tests: inspector/styles/styles-disable-then-change.html inspector/elements/elements-delete-inline-style.html inspector/styles/styles-update-from-js.html
WebKit Review Bot
Comment 3 2011-05-18 11:29:32 PDT
Created attachment 93950 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Alexander Pavlov (apavlov)
Comment 4 2011-05-19 09:48:01 PDT
Created attachment 94080 [details] [PATCH] A different approach applied
WebKit Review Bot
Comment 5 2011-05-19 09:53:12 PDT
Comment on attachment 94080 [details] [PATCH] A different approach applied Attachment 94080 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8716355
WebKit Review Bot
Comment 6 2011-05-19 09:56:46 PDT
Comment on attachment 94080 [details] [PATCH] A different approach applied Attachment 94080 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/8718226
Early Warning System Bot
Comment 7 2011-05-19 09:57:06 PDT
Comment on attachment 94080 [details] [PATCH] A different approach applied Attachment 94080 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/8716358
WebKit Review Bot
Comment 8 2011-05-19 10:45:53 PDT
Comment on attachment 94080 [details] [PATCH] A different approach applied Attachment 94080 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/8719129
Alexander Pavlov (apavlov)
Comment 9 2011-05-20 06:56:49 PDT
Created attachment 94214 [details] [PATCH] Offline comments addressed
Alexander Pavlov (apavlov)
Comment 10 2011-06-09 03:26:05 PDT
Created attachment 96567 [details] [PATCH] Rebase @reviewers: ping
Pavel Feldman
Comment 11 2011-06-15 04:49:32 PDT
Comment on attachment 96567 [details] [PATCH] Rebase View in context: https://bugs.webkit.org/attachment.cgi?id=96567&action=review A number of small nits and a test request. > Source/WebCore/inspector/Inspector.json:787 > + "id": "DOMNodeAttributes", Attributes. We can also rename DOMNode and DOMEventListener since type names have domain scope. > Source/WebCore/inspector/Inspector.json:790 > + { "name": "id", "type": "integer", "description": "Node id." }, Element id to get attributes for. > Source/WebCore/inspector/Inspector.json:987 > + "name": "getNodeAttributes", getAttributes > Source/WebCore/inspector/Inspector.json:1013 > + { "name": "nodeId", "type": "integer", "description": "Id of the node that has changed." } You might want to leave this as is for performance considerations. > Source/WebCore/inspector/Inspector.json:1018 > + "name": "inlineStyleInvalidated", Please add a test for this new event. > Source/WebCore/inspector/InspectorDOMAgent.cpp:255 > + m_domAgent->notifyStyleAttrInvalidated(elements); StyleAttributeInvalidated (no abbreaviations in WebCore)
Alexander Pavlov (apavlov)
Comment 12 2011-06-15 07:06:52 PDT
Comment on attachment 96567 [details] [PATCH] Rebase View in context: https://bugs.webkit.org/attachment.cgi?id=96567&action=review A fixed patch to be attached shortly >> Source/WebCore/inspector/Inspector.json:787 >> + "id": "DOMNodeAttributes", > > Attributes. We can also rename DOMNode and DOMEventListener since type names have domain scope. Done. I believe it's better to address the other renames in a separate change (in case we have to merge/roll back this or related changes). >> Source/WebCore/inspector/Inspector.json:790 >> + { "name": "id", "type": "integer", "description": "Node id." }, > > Element id to get attributes for. Done >> Source/WebCore/inspector/Inspector.json:987 >> + "name": "getNodeAttributes", > > getAttributes Done >> Source/WebCore/inspector/Inspector.json:1013 >> + { "name": "nodeId", "type": "integer", "description": "Id of the node that has changed." } > > You might want to leave this as is for performance considerations. This is one of the main points of this change. With the Flickr demo, you get a constant high-volume traffic of updated attributes ("style" in particular) the Web Inspector is not coping with (parsing + handling). >> Source/WebCore/inspector/Inspector.json:1018 >> + "name": "inlineStyleInvalidated", > > Please add a test for this new event. Will do >> Source/WebCore/inspector/InspectorDOMAgent.cpp:255 >> + m_domAgent->notifyStyleAttrInvalidated(elements); > > StyleAttributeInvalidated (no abbreaviations in WebCore) Done
Alexander Pavlov (apavlov)
Comment 13 2011-06-15 08:56:31 PDT
Created attachment 97301 [details] [PATCH] Comments addressed
Alexander Pavlov (apavlov)
Comment 14 2011-06-15 10:03:09 PDT
Ryosuke Niwa
Comment 15 2011-06-15 13:34:33 PDT
Ryosuke Niwa
Comment 16 2011-06-15 13:35:04 PDT
+mrobinson since this patch caused a test failure on GTK.
Note You need to log in before you can comment on or make changes to this bug.