Summary: | Avoid fetching JSObject::structure() repeatedly in putDirectInternal. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||
Component: | JavaScriptCore | Assignee: | Andreas Kling <kling> | ||||
Status: | RESOLVED WONTFIX | ||||||
Severity: | Normal | CC: | commit-queue, kling | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 130887 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Andreas Kling
2014-03-27 13:45:16 PDT
Created attachment 227974 [details]
Patch
Comment on attachment 227974 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227974&action=review r=me > Source/JavaScriptCore/runtime/JSObject.h:1346 > + if (Structure* newStructure = Structure::addPropertyTransitionToExistingStructure(this->structure(), propertyName, attributes, specificFunction, offset)) { s/this->structure()/structure/ right? Committed r166376: <http://trac.webkit.org/changeset/166376> Comment on attachment 227974 [details]
Patch
This really should have asserted, in all of the places that you removed a load of the structure, that the structure didn't change. That's very important since there are subtle ways in which things could *become* effectful and obviously this code could break if that happened.
(In reply to comment #4) > (From update of attachment 227974 [details]) > This really should have asserted, in all of the places that you removed a load of the structure, that the structure didn't change. That's very important since there are subtle ways in which things could *become* effectful and obviously this code could break if that happened. Sure, I'll slap that in there. Re-opened since this is blocked by bug 130887 (In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 227974 [details] [details]) > > This really should have asserted, in all of the places that you removed a load of the structure, that the structure didn't change. That's very important since there are subtle ways in which things could *become* effectful and obviously this code could break if that happened. > > Sure, I'll slap that in there. Ended up rolling this out instead. I had misread the profile that lead to making this change. |