RESOLVED FIXED 121735
Shrink attribute event listener code
https://bugs.webkit.org/show_bug.cgi?id=121735
Summary Shrink attribute event listener code
Darin Adler
Reported 2013-09-20 23:36:41 PDT
Shink attribute event listener code
Attachments
Patch (80.60 KB, patch)
2013-09-20 23:48 PDT, Darin Adler
koivisto: review+
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (466.55 KB, application/zip)
2013-09-21 00:50 PDT, Build Bot
no flags
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (493.66 KB, application/zip)
2013-09-21 01:26 PDT, Build Bot
no flags
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (469.18 KB, application/zip)
2013-09-21 01:56 PDT, Build Bot
no flags
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (537.93 KB, application/zip)
2013-09-21 02:08 PDT, Build Bot
no flags
Darin Adler
Comment 1 2013-09-20 23:48:30 PDT
WebKit Commit Bot
Comment 2 2013-09-20 23:49:34 PDT
Attachment 212256 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/UseJSC.cmake', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/bindings/js/JSBindingsAllInOne.cpp', u'Source/WebCore/bindings/js/JSDOMGlobalObject.h', u'Source/WebCore/bindings/js/JSLazyEventListener.cpp', u'Source/WebCore/bindings/js/JSLazyEventListener.h', u'Source/WebCore/bindings/js/ScriptEventListener.cpp', u'Source/WebCore/bindings/js/ScriptEventListener.h', u'Source/WebCore/dom/ContainerNode.cpp', u'Source/WebCore/dom/ContainerNode.h', u'Source/WebCore/dom/Document.cpp', u'Source/WebCore/dom/Document.h', u'Source/WebCore/html/HTMLBodyElement.cpp', u'Source/WebCore/html/HTMLButtonElement.cpp', u'Source/WebCore/html/HTMLElement.cpp', u'Source/WebCore/html/HTMLFormControlElement.cpp', u'Source/WebCore/html/HTMLFormElement.cpp', u'Source/WebCore/html/HTMLFrameElementBase.cpp', u'Source/WebCore/html/HTMLFrameSetElement.cpp', u'Source/WebCore/html/HTMLImageElement.cpp', u'Source/WebCore/html/HTMLInputElement.cpp', u'Source/WebCore/html/HTMLLinkElement.cpp', u'Source/WebCore/html/HTMLMediaElement.cpp', u'Source/WebCore/html/HTMLObjectElement.cpp', u'Source/WebCore/html/HTMLScriptElement.cpp', u'Source/WebCore/html/HTMLSelectElement.cpp', u'Source/WebCore/html/HTMLStyleElement.cpp', u'Source/WebCore/html/HTMLTextFormControlElement.cpp', u'Source/WebCore/html/HTMLTrackElement.cpp', u'Source/WebCore/html/parser/XSSAuditor.cpp', u'Source/WebCore/html/track/LoadableTextTrack.cpp', u'Source/WebCore/inspector/InspectorDOMAgent.cpp', u'Source/WebCore/svg/SVGElement.cpp', u'Source/WebCore/svg/SVGSVGElement.cpp', u'Source/WebCore/svg/SVGScriptElement.cpp']" exit_code: 1 Source/WebCore/bindings/js/JSLazyEventListener.h:29: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
EFL EWS Bot
Comment 3 2013-09-21 00:09:59 PDT
Antti Koivisto
Comment 4 2013-09-21 00:12:19 PDT
Comment on attachment 212256 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=212256&action=review > Source/WebCore/ChangeLog:3 > + Shink attribute event listener code Shrink > Source/WebCore/bindings/js/JSLazyEventListener.h:35 > + static PassRefPtr<JSLazyEventListener> createForNode(ContainerNode&, const QualifiedName& attributeName, const AtomicString& attributeValue); Future tightening to separate Document and Element paths might be beneficial.
Build Bot
Comment 5 2013-09-21 00:50:06 PDT
Comment on attachment 212256 [details] Patch Attachment 212256 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/2021182 New failing tests: inspector/elements/event-listener-sidebar.html
Build Bot
Comment 6 2013-09-21 00:50:09 PDT
Created attachment 212260 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Build Bot
Comment 7 2013-09-21 01:25:57 PDT
Comment on attachment 212256 [details] Patch Attachment 212256 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/2040081 New failing tests: inspector/elements/event-listener-sidebar.html
Build Bot
Comment 8 2013-09-21 01:26:00 PDT
Created attachment 212266 [details] Archive of layout-test-results from webkit-ews-07 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Build Bot
Comment 9 2013-09-21 01:56:47 PDT
Comment on attachment 212256 [details] Patch Attachment 212256 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/2010026 New failing tests: inspector/elements/event-listener-sidebar.html
Build Bot
Comment 10 2013-09-21 01:56:49 PDT
Created attachment 212268 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Build Bot
Comment 11 2013-09-21 02:08:52 PDT
Comment on attachment 212256 [details] Patch Attachment 212256 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1925586 New failing tests: inspector/elements/event-listener-sidebar.html
Build Bot
Comment 12 2013-09-21 02:08:55 PDT
Created attachment 212269 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Darin Adler
Comment 13 2013-09-21 10:31:36 PDT
Comment on attachment 212256 [details] Patch I’ll have to fix that event listener sidebar test failure. Seems like I made some mistake in the inspector part of this.
Darin Adler
Comment 14 2013-09-21 11:24:33 PDT
Darin Adler
Comment 15 2013-09-21 20:35:17 PDT
(In reply to comment #4) > > Source/WebCore/ChangeLog:3 > > + Shink attribute event listener code > > Shrink Oops. Forgot to fix that! > > Source/WebCore/bindings/js/JSLazyEventListener.h:35 > > + static PassRefPtr<JSLazyEventListener> createForNode(ContainerNode&, const QualifiedName& attributeName, const AtomicString& attributeValue); > > Future tightening to separate Document and Element paths might be beneficial. Good idea. I’ll do that later.
Andreas Kling
Comment 16 2013-09-22 07:24:58 PDT
This looks like it may have caused a 2.5%+ regression on html5-full-render on ML on perf.webkit.org: https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac-mountainlion%22%2C%22Parser%2Fhtml5-full-render%3ATime%22%5D%5D
Darin Adler
Comment 17 2013-09-22 15:54:55 PDT
Ouch! Well, I assume we can get that speed back with inlining.
Darin Adler
Comment 18 2013-09-22 15:55:10 PDT
Which might or might not undo the shrinking.
Note You need to log in before you can comment on or make changes to this bug.