[Webkit-unassigned] [Bug 162124] Unite op_get_by_id_with_this with op_get_by_id

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=162124

--- Comment #14 from Caio Lima <ticaiolima at gmail.com> ---
(In reply to comment #13)
> Comment on attachment 289377 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289377&action=review
> 
> > Source/JavaScriptCore/ChangeLog:11
> > +        This patch is merging the op_get_by_id_with_this with op_get_by_id and
> > +        this way we enable get_by_id optimizations such as Monomorphic/Polymorphic Inline
> > +        Cache on JIT layers for super member access. These optimizations is improving access
> > +        of super members in ~20%.
> 
> Fully unoptimized property access is 10X slower than fully optimized
> property access. Therefore, it's surprising that this patch is not a bigger
> win on super-get-by-id-with-this-monomorphic. It looks like
> super-get-by-id-with-this-monomorphic is a getter/setter benchmark. Can you
> verify that the FTL successfully inlines the getter and setter for value()?
> Perhaps the win on this benchmark is not bigger because the benchmark
> includes an intermediate "calc" function, which is pretty expensive. Can you
> report the speedup on this benchmark if you remove the call to calc?

Good to know that. I faced a problem while creating the patch to get GetById compiled in FTL layer. To get it working, I was using --accessInline=false. What I mean is that the reason can br related with that. I am going to investigate it.

> > Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp:1096
> > -            GPRReg baseForCustomValue = m_type == CustomValueGetter || m_type == CustomValueSetter ? baseForAccessGPR : baseForGetGPR;
> > +            GPRReg baseForCustomValue = m_type == CustomValueGetter || m_type == CustomValueSetter ? baseForAccessGPR : baseForCustomGetGPR;
> 
> Isn't the answer always thisGPR? Under what conditions do I want to invoke a
> getter or setter without the specified this?

If I am not wrong, no. This code is dominated by this manipulation over baseForGetGPR (https://github.com/caiolima/webkit/blob/merge_by_ids/Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp#L879). So, when thisGPR == baseGPR, it is safer get baseForGetGPR because of "Proxy observed" lookups. I am thinking that it is worth think in cases when thisOperand != baseOperand and it is "Proxy Observed". Does it make sense?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160920/95252e45/attachment-0001.html>