[webkit-reviews] review granted: [Bug 117377] fourthTier: DFG GetById patching shouldn't distinguish between self lists and proto lists : [Attachment 204106] the patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 117377: fourthTier: DFG GetById patching shouldn't distinguish between self
lists and proto lists
https://bugs.webkit.org/show_bug.cgi?id=117377

Attachment 204106: the patch
https://bugs.webkit.org/attachment.cgi?id=204106&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=204106&action=review


r=me, but I think I spotted a bug.

> Source/JavaScriptCore/ChangeLog:17
> +	   I believe that this was introduced way back in the days of the old
interpreter,
> +	   where distinguishing between self lists, proto lists, and chain
lists was meaningful
> +	   for interpreter performance: it meant fewer branches to evaluate
those lists. Then

Yup.

> Source/JavaScriptCore/bytecode/PolymorphicAccessStructureList.h:69
> -	       this->isDirect = isDirect;
> +	       isDirect = _isDirect;

I prefer the this-> style, but I guess it's the minority in this function.

> Source/JavaScriptCore/dfg/DFGRepatch.cpp:423
> +	   || baseValue.asCell()->structure()->isUncacheableDictionary())

I think we need to check propertyAccessesAreCacheable() instead of
isUncacheableDictionary(), to account for prohibitsPropertyCaching().