Bug 103708 - Simplify treeScope and setTreeScope
Summary: Simplify treeScope and setTreeScope
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: Elliott Sprehn
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-29 20:35 PST by Elliott Sprehn
Modified: 2012-12-06 17:06 PST (History)
10 users (show)

See Also:


Attachments
Patch (8.18 KB, patch)
2012-11-29 22:09 PST, Elliott Sprehn
no flags Details | Formatted Diff | Diff
Patch (9.78 KB, patch)
2012-11-30 14:54 PST, Elliott Sprehn
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Elliott Sprehn 2012-11-29 20:35:58 PST
Make treeScope inline and remove a branch
Comment 1 Elliott Sprehn 2012-11-29 22:09:11 PST
Created attachment 176899 [details]
Patch
Comment 2 Elliott Sprehn 2012-11-29 22:19:57 PST
Doh, seems we need to put the inline version in Document.h
Comment 3 Early Warning System Bot 2012-11-29 22:20:34 PST
Comment on attachment 176899 [details]
Patch

Attachment 176899 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15054297
Comment 4 Early Warning System Bot 2012-11-29 22:23:55 PST
Comment on attachment 176899 [details]
Patch

Attachment 176899 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15067013
Comment 5 EFL EWS Bot 2012-11-29 22:32:08 PST
Comment on attachment 176899 [details]
Patch

Attachment 176899 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15054301
Comment 6 WebKit Review Bot 2012-11-29 22:35:05 PST
Comment on attachment 176899 [details]
Patch

Attachment 176899 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15056256
Comment 7 kov's GTK+ EWS bot 2012-11-29 22:39:32 PST
Comment on attachment 176899 [details]
Patch

Attachment 176899 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15059187
Comment 8 Hajime Morrita 2012-11-29 23:21:52 PST
Comment on attachment 176899 [details]
Patch

This is nice!
Comment 9 Build Bot 2012-11-29 23:44:01 PST
Comment on attachment 176899 [details]
Patch

Attachment 176899 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15055300
Comment 10 Peter Beverloo (cr-android ews) 2012-11-30 10:50:50 PST
Comment on attachment 176899 [details]
Patch

Attachment 176899 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15061399
Comment 11 Elliott Sprehn 2012-11-30 14:54:40 PST
Created attachment 177030 [details]
Patch
Comment 12 Hajime Morrita 2012-11-30 15:06:44 PST
Comment on attachment 177030 [details]
Patch

Looks good. Please wait until non-Chromium bots become green.
Comment 13 WebKit Review Bot 2012-12-02 03:17:27 PST
Comment on attachment 177030 [details]
Patch

Clearing flags on attachment: 177030

Committed r136328: <http://trac.webkit.org/changeset/136328>
Comment 14 WebKit Review Bot 2012-12-02 03:17:31 PST
All reviewed patches have been landed.  Closing bug.
Comment 15 Ojan Vafai 2012-12-03 16:49:05 PST
I think this may have caused a ~12% improvement in dom_perf/CreateNodes. Hard to tell. http://trac.webkit.org/changeset/136334 seems more likely.

http://build.chromium.org/f/chromium/perf/linux-release-webkit-latest/dom_perf/report.html?graph=CreateNodes&trace=score&rev=170979&history=150
Comment 16 Elliott Sprehn 2012-12-03 16:54:34 PST
(In reply to comment #15)
> I think this may have caused a ~12% improvement in dom_perf/CreateNodes. Hard to tell. http://trac.webkit.org/changeset/136334 seems more likely.
> 

Hmm, that change is going to really skew our perf benchmarks that create nodes because they all run inside the 10 second window. They don't represent what goes on inside a real webapp which you keep open for a while.
Comment 17 Beth Dakin 2012-12-06 17:00:29 PST
I think that this change is causing this web page to assert: http://www2.macleans.ca/2012/11/04/tarnished-silver-assessing-the-new-king-of-stats/


Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x000000010f9b3094 WebCore::Node::rareData() const + 84 (Node.cpp:484)
1   com.apple.WebCore             	0x000000010f9b302d WebCore::Node::setTreeScope(WebCore::TreeScope*) + 141 (Node.cpp:451)
2   com.apple.WebCore             	0x000000011016d1d0 WebCore::TreeScopeAdopter::moveTreeToNewScope(WebCore::Node*) const + 288 (TreeScopeAdopter.cpp:53)
3   com.apple.WebCore             	0x000000011016a668 WebCore::TreeScopeAdopter::execute() const + 24 (TreeScopeAdopter.h:38)
4   com.apple.WebCore             	0x0000000110169a77 WebCore::TreeScope::adoptIfNeeded(WebCore::Node*) + 391 (TreeScope.cpp:269)
5   com.apple.WebCore             	0x000000010ea09274 WebCore::Private::NodeRemovalDispatcher<WebCore::Node, WebCore::ContainerNode, true>::dispatch(WebCore::Node*, WebCore::ContainerNode*) + 68 (ContainerNodeAlgorithms.h:141)
6   com.apple.WebCore             	0x000000010ea091eb void WebCore::Private::addChildNodesToDeletionQueue<WebCore::Node, WebCore::ContainerNode>(WebCore::Node*&, WebCore::Node*&, WebCore::ContainerNode*) + 331 (ContainerNodeAlgorithms.h:183)
7   com.apple.WebCore             	0x000000010ea069c0 void WebCore::removeAllChildrenInContainer<WebCore::Node, WebCore::ContainerNode>(WebCore::ContainerNode*) + 48 (ContainerNodeAlgorithms.h:91)
8   com.apple.WebCore             	0x000000010ea01ba5 WebCore::ContainerNode::removeAllChildren() + 21 (ContainerNode.cpp:94)
9   com.apple.WebCore             	0x000000010f1cebe4 WebCore::InputType::destroyShadowSubtree() + 68 (InputType.cpp:492)
10  com.apple.WebCore             	0x00000001101217df WebCore::TextFieldInputType::destroyShadowSubtree() + 31 (TextFieldInputType.cpp:308)

etc.

I will file a bug.
Comment 18 Beth Dakin 2012-12-06 17:06:29 PST
https://bugs.webkit.org/show_bug.cgi?id=104326