Make setting of accessors even more atomic.

Make setting of accessors even more atomic.

Now the whole getter/setter/attributes triple gets created/set together,
avoiding any hacks regarding previous values/attributes, making things a lot
simpler.

While doing this, an interesting problem surfaced, which has been there for a
long time: After adding/changing acessors in slow mode, we could potentially
fail going back to fast mode because of a failed memory allocation, signaling
the need for a GC. But we have already changed the object in slow mode, so we
are not idempotent and the retry would trigger a newly inserted assertion
(namely, that the code obeys access restrictions). This has been solved by
splitting the transformation to fast mode from the actual setting of the
accessors.

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

[Michael Starzinger, 2012-03-21 12:33:08]

https://chromiumcodereview.appspot.com/9716035/diff/1/src/api.cc
File src/api.cc (right):

https://chromiumcodereview.appspot.com/9716035/diff/1/src/api.cc#newcode3069
src/api.cc:3069: i::JSObject::TransformToFastProperties(Utils::OpenHandle(this),
0);
It would be nice not to have this code duplicated in api.cc and runtime.cc, but
I have no suggestion how to make it cleaner ATM.

https://chromiumcodereview.appspot.com/9716035/diff/1/src/objects.cc
File src/objects.cc (left):

https://chromiumcodereview.appspot.com/9716035/diff/1/src/objects.cc#oldcode4517
src/objects.cc:4517: bool convert_back_to_fast = HasFastProperties() &&
Here we only convert the object back to fast properties when it had fast
properties before. With the current change we will always convert back to fast
properties after defining an accessor. I am not sure this what we want.

https://chromiumcodereview.appspot.com/9716035/diff/1/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/9716035/diff/1/src/objects.h#newcode8054
src/objects.h:8054: Object* SafeGet(AccessorComponent component);
Since there is no "unsafe" version anymore, can we just call this GetComponent()
or Get()?

https://chromiumcodereview.appspot.com/9716035/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://chromiumcodereview.appspot.com/9716035/diff/1/src/runtime.cc#newcode4354
src/runtime.cc:4354: JSObject::TransformToFastProperties(obj, 0);
See first comment.

[Sven Panne, 2012-03-21 13:14:18]

Review comments addressed, please take another look...

http://codereview.chromium.org/9716035/diff/1/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/9716035/diff/1/src/api.cc#newcode3069
src/api.cc:3069: i::JSObject::TransformToFastProperties(Utils::OpenHandle(this),
0);
On 2012/03/21 12:33:08, Michael Starzinger wrote:
> It would be nice not to have this code duplicated in api.cc and runtime.cc,
but
> I have no suggestion how to make it cleaner ATM.

Well, C++'s abstraction capabilities are extremely restricted, you basically
have to define a class for everything. In our concrete example, we would need a
bracket-like structure (using Andreas' favorite strategy pattern ;-), keeping
the previous "fastness" as the local state. Not complicated, but a kind of a
sledge-hammer for this, so I'll leave this as-is (as discussed offline).

http://codereview.chromium.org/9716035/diff/1/src/objects.cc
File src/objects.cc (left):

http://codereview.chromium.org/9716035/diff/1/src/objects.cc#oldcode4517
src/objects.cc:4517: bool convert_back_to_fast = HasFastProperties() &&
On 2012/03/21 12:33:08, Michael Starzinger wrote:
> Here we only convert the object back to fast properties when it had fast
> properties before. With the current change we will always convert back to fast
> properties after defining an accessor. I am not sure this what we want.

This was an accidental change in semantics, I don't think we always want to
convert to fast mode, e.g. in the case of objects used as hash maps. I'll add
the right logic to the callers. Done.

http://codereview.chromium.org/9716035/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/9716035/diff/1/src/objects.h#newcode8054
src/objects.h:8054: Object* SafeGet(AccessorComponent component);
On 2012/03/21 12:33:08, Michael Starzinger wrote:
> Since there is no "unsafe" version anymore, can we just call this
GetComponent()
> or Get()?

I think I'll go for GetComponent, sounds like the right counterpart for
SetComponents. Done.

http://codereview.chromium.org/9716035/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/9716035/diff/1/src/runtime.cc#newcode4354
src/runtime.cc:4354: JSObject::TransformToFastProperties(obj, 0);
On 2012/03/21 12:33:08, Michael Starzinger wrote:
> See first comment.

Done.

[Michael Starzinger, 2012-03-23 09:01:26]

LGTM.