Begin to handlify the stub cache.

Begin to handlify the stub cache.

Handlify the first layer of the stub cache functions for looking up LoadICs,
and make them able to perform GC on allocation failures.

There was an implicit handle scope created in by the stub compilers in the
cache of a stub cache lookup miss.  This is replaced by an explicit handle
scope so that it can be moved earlier in the case of LoadICs.

[Kevin Millikin (Chromium), 2011-09-29 09:21:32]

Request for comments.  Consider this a proof of concept to see if we can convert
one horizontal layer at a time (from the outermost) and one vertical slice
(here, LoadIC).

This seems like a big, manual project.  Is it worth doing?

Advantages: simpler code, fewer bugs.  Ultimately, we can get rid of some code
(like the assembler functions that try to call a stub and signal failure if they
can't compile it).  We can eventually mark these stubs as safe for compiling
another stub while generating.

Disadvantages: a handle scope in cases where we didn't have one before, namely
whenever we decide to patch an IC instead of just when we have to compiler.  A
constant overhead due to a layer of indirection.

http://codereview.chromium.org/8068026/diff/1/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/8068026/diff/1/src/ic.cc#newcode987
src/ic.cc:987: HandleScope scope(isolate());
Before, we created this handle scope only in the case of a stub cache lookup
miss, when we actually compiled a stub.  Now, we always create it here.

That's a bit more overhead but we've already gone to the runtime and decided to
patch code, so I don't think it will be significant.

http://codereview.chromium.org/8068026/diff/1/src/stub-cache.cc
File src/stub-cache.cc (left):

http://codereview.chromium.org/8068026/diff/1/src/stub-cache.cc#oldcode134
src/stub-cache.cc:134: LoadStubCompiler compiler;
Though this implicitly introduces a handle scope, all the code uses raw pointers
and so can't tolerate a GC.  So we have to check for allocation failures
explicitly at every level.

We've had a lot of bugs where we get a GC several layers deep (e.g., in the
assembler), which cuts intercepts the allocation failure that the outer layers
needed to safely unwind.

http://codereview.chromium.org/8068026/diff/1/src/stub-cache.cc
File src/stub-cache.cc (right):

http://codereview.chromium.org/8068026/diff/1/src/stub-cache.cc#newcode119
src/stub-cache.cc:119: CALL_HEAP_FUNCTION(
This is the interface between handle code and raw pointer code, that will
perform GC and retry on allocation failures.  The handle code can tolerate the
GC.

http://codereview.chromium.org/8068026/diff/1/src/stub-cache.cc#newcode149
src/stub-cache.cc:149: // we could avoid the linear space usage.
We never do this trick because we're paranoid about handle cell ownership and
mutating them behind the scenes (it would be really bad if we mutated a
'spoofed' handle like one of the heap roots).

[danno, 2011-09-29 11:00:05]

I agree that having the code Handle-based would cause less grief overall, and
handle-ifying the stub cache has been in the TODO doc for a while. It seems
reasonable that when the work is done, that is is done incremental by vertical
and horizontal slice as you suggest. 

However, I do have strong reservations about doing that work now. I am concerned
that in the transition period it will be tricky to make sure that the places
that have mixed pointer/handle code are not harboring latent bugs. I think when
we do this it needs to be done as swiftly as possible, so it needs to have a
planned priority. That priority is not high enough right now.

[fschneider, 2011-10-12 13:11:18]

+1 for handlifying the stub compiler code.

Maybe we can just do this in one big commit to avoid having a mix of both world
in different places?

[ulan, 2011-10-13 17:36:09]

http://codereview.chromium.org/8068026/diff/1/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/8068026/diff/1/src/ic.cc#newcode947
src/ic.cc:947: UpdateCaches(&lookup, state, object, name);
Can lookup.holder() become stale if this call causes GC?

I've been getting flaky craches until I handlified it.

[fschneider, 2011-10-14 07:42:11]

http://codereview.chromium.org/8068026/diff/1/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/8068026/diff/1/src/ic.cc#newcode947
src/ic.cc:947: UpdateCaches(&lookup, state, object, name);
On 2011/10/13 17:36:10, ulan wrote:
> Can lookup.holder() become stale if this call causes GC?
> 
> I've been getting flaky craches until I handlified it.

I think you're right, because LookupResult has a raw pointer to the object and
it is used after this call (e.g. passed into GetProperty)