Bug 133558

Summary: Eagerly reify DOM prototype attributes
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: bunhere, cdumez, cgarcia, clopez, commit-queue, gyuyoung.kim, kling, rakuco, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch oliver: review+

Mark Hahnenberg
Reported 2014-06-05 13:25:36 PDT
This allows us to get rid of a lot of the additional overhead of pushing DOM attributes up into the prototype. By eagerly reifying the custom getters and setters into the actual JSObject we avoid having to override getOwnPropertySlot for all of the DOM prototypes, which is a lot of the overhead of doing property lookups on DOM wrappers.
Attachments
Patch (57.45 KB, patch)
2014-06-05 13:36 PDT, Mark Hahnenberg
no flags
Patch (57.45 KB, patch)
2014-06-05 13:43 PDT, Mark Hahnenberg
no flags
Patch (59.12 KB, patch)
2014-06-05 14:01 PDT, Mark Hahnenberg
no flags
Patch (59.56 KB, patch)
2014-06-05 14:23 PDT, Mark Hahnenberg
oliver: review+
Mark Hahnenberg
Comment 1 2014-06-05 13:36:30 PDT
WebKit Commit Bot
Comment 2 2014-06-05 13:39:19 PDT
Attachment 232580 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/CustomGetterSetter.h:72: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Hahnenberg
Comment 3 2014-06-05 13:43:35 PDT
WebKit Commit Bot
Comment 4 2014-06-05 13:46:26 PDT
Attachment 232581 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/CustomGetterSetter.h:72: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Hahnenberg
Comment 5 2014-06-05 14:01:53 PDT
WebKit Commit Bot
Comment 6 2014-06-05 14:03:29 PDT
Attachment 232582 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/CustomGetterSetter.h:72: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 36 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Hahnenberg
Comment 7 2014-06-05 14:23:39 PDT
WebKit Commit Bot
Comment 8 2014-06-05 14:24:35 PDT
Attachment 232583 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/CustomGetterSetter.h:72: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
Oliver Hunt
Comment 9 2014-06-05 18:38:19 PDT
Comment on attachment 232583 [details] Patch You'll need to do run-bindings-tests verify that the diffs look sane, then run-bindings-tests --reset-results otherwise yay!
Andreas Kling
Comment 10 2014-06-07 10:04:45 PDT
This doesn't work for HTMLDListElement and HTMLOListElement. They crap out in InterfaceRequiresAttributesOnInstance because they have "List" in the name. If you change that line to something like this instead: return 1 if $interfaceName =~ /List$/; ..then this will go even faster, as the jslib test pages have a bunch of these elements. :) Other than that, truly awesome stuff.
Mark Hahnenberg
Comment 11 2014-06-09 11:54:08 PDT
Mark Hahnenberg
Comment 12 2014-06-09 12:31:59 PDT
Alexey Proskuryakov
Comment 13 2014-06-09 15:31:29 PDT
js/dom/global-constructors-attributes-dedicated-worker.html started to crash every time in debug builds after this. Mark is working on a fix. http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=js%2Fdom%2Fglobal-constructors-attributes-dedicated-worker.html
Mark Hahnenberg
Comment 14 2014-06-09 15:39:04 PDT
The regression was actually a pre-existing bug that this change revealed, so I filed bug 133661 to track it separately.
Carlos Alberto Lopez Perez
Comment 15 2014-06-18 03:49:42 PDT
(In reply to comment #11) > Committed r169703: <http://trac.webkit.org/changeset/169703> This caused a regression of ~50% on memory usage (JSHeap) on all performance tests: https://bugs.webkit.org/show_bug.cgi?id=133876
Note You need to log in before you can comment on or make changes to this bug.