Restructure JSObject::SetElement for performance.

Restructure JSObject::SetElement for performance.

Wins back ~1500 points on Octane/Gameboy that we lost with
https://codereview.chromium.org/11365111 (CL 12900), presumably
by lowering register pressure and/or handlification overhead.
Hopefully benefits other regressions as well.

R=verwaest@chromium.org
BUG=

Committed: https://code.google.com/p/v8/source/detail?r=12949

[rafaelw, 2012-11-13 13:18:44]

https://codereview.chromium.org/11275283/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/11275283/diff/1/src/objects.cc#newcode10310
src/objects.cc:10310: return HasIndexedInterceptor()
consider factoring out into inline static?

https://codereview.chromium.org/11275283/diff/1/src/objects.cc#newcode10344
src/objects.cc:10344: if (old_attributes == ABSENT) {
nit: return early here. e.g.

if (old_attributes != ABSENT)
  return *hresult;

EnqueueChangeRecord...

[Toon Verwaest, 2012-11-13 13:38:08]

lgtm with nit

https://codereview.chromium.org/11275283/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/11275283/diff/1/src/objects.cc#newcode10310
src/objects.cc:10310: return HasIndexedInterceptor()
+1
On 2012/11/13 13:18:44, rafaelw wrote:
> consider factoring out into inline static?

https://codereview.chromium.org/11275283/diff/1/src/objects.cc#newcode10344
src/objects.cc:10344: if (old_attributes == ABSENT) {
On 2012/11/13 13:18:44, rafaelw wrote:
> nit: return early here. e.g.
> 
> if (old_attributes != ABSENT)
>   return *hresult;
> 
> EnqueueChangeRecord...

This doesn't seem possible, given that the value is possibly updated, or the
property was reconfigured.

[rafaelw, 2012-11-13 13:41:25]

https://codereview.chromium.org/11275283/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/11275283/diff/1/src/objects.cc#newcode10344
src/objects.cc:10344: if (old_attributes == ABSENT) {
You are correct. Sorry. Misread the code.

On 2012/11/13 13:38:08, Toon Verwaest wrote:
> On 2012/11/13 13:18:44, rafaelw wrote:
> > nit: return early here. e.g.
> > 
> > if (old_attributes != ABSENT)
> >   return *hresult;
> > 
> > EnqueueChangeRecord...
> 
> This doesn't seem possible, given that the value is possibly updated, or the
> property was reconfigured.

[rossberg, 2012-11-13 13:47:44]

https://chromiumcodereview.appspot.com/11275283/diff/1/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/11275283/diff/1/src/objects.cc#newcode...
src/objects.cc:10310: return HasIndexedInterceptor()
On 2012/11/13 13:38:08, Toon Verwaest wrote:
> +1
> On 2012/11/13 13:18:44, rafaelw wrote:
> > consider factoring out into inline static?

Note that one version is handlified while the other is not, which is a central
point of the whole exercise.

[rafaelw, 2012-11-13 14:53:01]

https://chromiumcodereview.appspot.com/11275283/diff/1/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/11275283/diff/1/src/objects.cc#newcode...
src/objects.cc:10310: return HasIndexedInterceptor()
I'm not understanding why it wouldn't work, but I defer to your judgement here.

On 2012/11/13 13:47:44, rossberg wrote:
> On 2012/11/13 13:38:08, Toon Verwaest wrote:
> > +1
> > On 2012/11/13 13:18:44, rafaelw wrote:
> > > consider factoring out into inline static?
> 
> Note that one version is handlified while the other is not, which is a central
> point of the whole exercise.

[rossberg, 2012-11-13 15:33:13]

https://chromiumcodereview.appspot.com/11275283/diff/1/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/11275283/diff/1/src/objects.cc#newcode...
src/objects.cc:10310: return HasIndexedInterceptor()
On 2012/11/13 14:53:01, rafaelw wrote:
> I'm not understanding why it wouldn't work, but I defer to your judgement
here.

Thing is, I could only factor it out in form of a handlified version, but then I
likely lose part of the performance I just tried to regain on the "normal" path.