[Isolates] Avoid dereferencing Isolate::Current() to check oddball identities...

[Isolates] Avoid dereferencing Isolate::Current() to check oddball identities.

- Added a kind field and accessors to JS Oddball objects.
- IsTrue/IsFalse/IsBoolean/IsTheHole/IsNull/IsUndefined now look like
  [IsOddball() && Oddball::cast(this)->kind() == Oddball::kUndefined;]
- Some checks of the form [if (foo == HEAP->an_oddball_value())]
  were updated (in such a way that they are resistant to foo being null
  and to the oddballs changing locations after a GC.)
- Removed THIS from heap.cc.


Committed: http://code.google.com/p/v8/source/detail?r=5191

[zarko, 2010-07-16 20:22:17]

Thanks
  Luke

[zarko, 2010-07-19 21:02:17]

On 2010/07/16 20:22:17, zarko wrote:
> Thanks
>   Luke

It turns out that the Is{True,False,...} tests don't require that [this] is
non-NULL. This is because the first check they perform is IsOddball(), which in
turn checks IsHeapObject(), which proceeds to do a bit test against [this] which
will fail for NULL pointers.

[Vitaly Repeshko, 2010-08-02 12:49:47]

LG with the comparison issue addressed.


-- Vitaly

http://codereview.chromium.org/3031005/diff/52001/53001
File src/accessors.cc (right):

http://codereview.chromium.org/3031005/diff/52001/53001#newcode44
src/accessors.cc:44: if (obj == heap->null_value()) return NULL;
Here and in lots of other places you're still using comparison to
heap->null_value(). Even if there are no other heap usages around. Why can't
these comparisons be replaced with obj->IsNull()?

http://codereview.chromium.org/3031005/diff/52001/53014
File src/objects-inl.h (right):

http://codereview.chromium.org/3031005/diff/52001/53014#newcode525
src/objects-inl.h:525: return IsOddball() && !(Oddball::cast(this)->kind() &
Oddball::kBooleanMask);
Use explicit "== 0" comparison.

http://codereview.chromium.org/3031005/diff/52001/53016
File src/objects.h (right):

http://codereview.chromium.org/3031005/diff/52001/53016#newcode4782
src/objects.h:4782: static const byte kBooleanMask = ~1;
Boolean mask should be "1". "~1" is NotBoolean mask.

[zarko, 2010-08-05 20:38:39]

http://codereview.chromium.org/3031005/diff/52001/53001
File src/accessors.cc (right):

http://codereview.chromium.org/3031005/diff/52001/53001#newcode44
src/accessors.cc:44: if (obj == heap->null_value()) return NULL;
On 2010/08/02 12:49:47, Vitaly wrote:
> Here and in lots of other places you're still using comparison to
> heap->null_value(). Even if there are no other heap usages around. Why can't
> these comparisons be replaced with obj->IsNull()?

It's likely cheaper to go through this sort of comparison in a loop as it would
involve a smaller net number of indirections.