Age inline caches after context disposal.

Age inline caches after context disposal.

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

[Vyacheslav Egorov (Chromium), 2012-03-22 14:47:05]

initial comments

https://chromiumcodereview.appspot.com/9837005/diff/1/src/heap.cc
File src/heap.cc (right):

https://chromiumcodereview.appspot.com/9837005/diff/1/src/heap.cc#newcode4854
src/heap.cc:4854: NotifyGlobalContextDisposed();
Why do we signal context disposal here and abort incremental marking?

What kind of hint Chrome passes to us?

https://chromiumcodereview.appspot.com/9837005/diff/1/src/incremental-marking.cc
File src/incremental-marking.cc (right):

https://chromiumcodereview.appspot.com/9837005/diff/1/src/incremental-marking...
src/incremental-marking.cc:185: IC::Clear(rinfo->pc());
I would expect we reset counter to be equal to
heap_->global_context_disposed_count() otherwise we would flush again on the
next GC even if the context was not disposed.

surely reset should not happen in the place but at the place which calls
visitor.

https://chromiumcodereview.appspot.com/9837005/diff/1/src/mark-compact.cc
File src/mark-compact.cc (right):

https://chromiumcodereview.appspot.com/9837005/diff/1/src/mark-compact.cc#new...
src/mark-compact.cc:1055: IC::Clear(rinfo->pc());
I would expect we reset counter to be equal to
heap_->global_context_disposed_count() otherwise we would flush again on the
next GC even if the context was not disposed.

surely reset should not happen in the place but at the place which calls
visitor.

https://chromiumcodereview.appspot.com/9837005/diff/1/src/mark-compact.cc#new...
src/mark-compact.cc:1412:
shared->set_context_disposed_count(heap->global_context_disposed_count());
I suppose this piece of code is here to prevent optimizer from optimizing to
early. 

There are heuristics that try to decide whether function should be optimized
based on the amount of initialized ICs.

Why do we need to reset opt_count separately? I would expect initialized ICs
heuristic to cover it.

Also we were discussing with Jakob if we should move opt_count to the code
object itself. This would allow to easily adjust it when we clear ICs instead of
doing special (spooky) handling of shared function info.

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

https://chromiumcodereview.appspot.com/9837005/diff/1/src/objects.h#newcode4436
src/objects.h:4436: static const int kFlagsOffset = kContextDisposedCountOffset
+ kIntSize;
I wonder how big our Code object will become with an addition of this one field.

I think it will grow by 50% because of the alignment.

[ulan, 2012-03-23 10:47:13]

Uploaded new patch set.

https://chromiumcodereview.appspot.com/9837005/diff/1/src/heap.cc
File src/heap.cc (right):

https://chromiumcodereview.appspot.com/9837005/diff/1/src/heap.cc#newcode4854
src/heap.cc:4854: NotifyGlobalContextDisposed();
On 2012/03/22 14:47:05, Vyacheslav Egorov wrote:
> Why do we signal context disposal here and abort incremental marking?
> 
> What kind of hint Chrome passes to us?
Removed aborting of incremental marking and added a comment about the hint.

https://chromiumcodereview.appspot.com/9837005/diff/1/src/incremental-marking.cc
File src/incremental-marking.cc (right):

https://chromiumcodereview.appspot.com/9837005/diff/1/src/incremental-marking...
src/incremental-marking.cc:185: IC::Clear(rinfo->pc());
On 2012/03/22 14:47:05, Vyacheslav Egorov wrote:
> I would expect we reset counter to be equal to
> heap_->global_context_disposed_count() otherwise we would flush again on the
> next GC even if the context was not disposed.
> 
> surely reset should not happen in the place but at the place which calls
> visitor.

As discussed offline, this should work because we flush stub caches on GC

https://chromiumcodereview.appspot.com/9837005/diff/1/src/mark-compact.cc
File src/mark-compact.cc (right):

https://chromiumcodereview.appspot.com/9837005/diff/1/src/mark-compact.cc#new...
src/mark-compact.cc:1055: IC::Clear(rinfo->pc());
On 2012/03/22 14:47:05, Vyacheslav Egorov wrote:
> I would expect we reset counter to be equal to
> heap_->global_context_disposed_count() otherwise we would flush again on the
> next GC even if the context was not disposed.
> 
> surely reset should not happen in the place but at the place which calls
> visitor.

As discussed offline, this should work because we flush stub caches on GC

https://chromiumcodereview.appspot.com/9837005/diff/1/src/mark-compact.cc#new...
src/mark-compact.cc:1412:
shared->set_context_disposed_count(heap->global_context_disposed_count());
This is to prevent us from marking a function as non-optimizable after N page
reloads when the opt_count reaches kMaxOptCount.

I will submit this part as a separate CL.

On 2012/03/22 14:47:05, Vyacheslav Egorov wrote:
> I suppose this piece of code is here to prevent optimizer from optimizing to
> early. 
> 
> There are heuristics that try to decide whether function should be optimized
> based on the amount of initialized ICs.
> 
> Why do we need to reset opt_count separately? I would expect initialized ICs
> heuristic to cover it.
> 
> Also we were discussing with Jakob if we should move opt_count to the code
> object itself. This would allow to easily adjust it when we clear ICs instead
of
> doing special (spooky) handling of shared function info.

[Vyacheslav Egorov (Chromium), 2012-03-23 13:14:52]

lgtm