Yeah there is something weird going on here. I noticed that one of the remaining cctests which fail for Ignition is test-heap/CompilationCacheCachingBehavior which is testing the in-memory code cache.

OK, so the issue here is that the compilation cache is relying on code aging to decide when to evict cache entries. The code CompilationCacheTable::Age() does the following:

   if (info->code()->kind() != Code::FUNCTION || info->code()->IsOld()) {
     // remove entry
   }

Therefore, any SFI which has bytecode will have it's code() pointing to the InterpreterEntryTrampoline builtin (not Code::FUNCTION) and will be removed whenever there is a GC.

It would be easy to change this so that if the SFI has bytecode then we shouldn't evict the entry, however if we do that, since we don't have a code aging mechanism on bytecode, the compilation cache will never clear out entries and we will just leak memory.

The only two thoughts I have are to either move back to the pre-code-aging implementation of the code cache (i.e., revert https://codereview.chromium.org/675013004) or to implement some form of code aging for bytecode.

Toon / Ulan / Michi - any thoughts on what the best solution for this would be?

I think proper code aging for bytecode would be the best of these three alternatives.

IIRC, 675013004 improved performance on speedometer or sunspider. 

I chatted to Michi about this and we agree we should implement code aging for bytecode to allow this. The idea is that we would age bytecode during GC, but NOT do code-flushing, this code aging would only be used to evict entries from the in-memory code cache.

To implement this I intend to add a custom StaticVisitor<StaticVisitor>::VisitBytecodeArray(..) which marks the bytecode array as being a generation older on each mark-compact GC, and have the InterpreterEntryTrampoline reset the mark to zero when the function is called.

How does this sound to the GC folks? Also, question for the GC folks, there seems to be a StaticNewSpaceVisitor<StaticVisitor>::VisitBytecodeArray() (and the same for SharedFunctionInfo), however both these objects should only ever be created tenured - is there a reason we need new space visitors for these objects?

Code-aging: Would this be a local modification to the byte code array or do you need to add them to a global state somewhere? We are trying hard to get the marking visitors side-effect free and this could make it even harder. I see why you want to have code aging though, so we should find a way in any case.

The StaticNewSpaceVisitor should have some UNREACHABLE() handlers for byte code arrays and SFI (Similar to VisitCodeEntry but they could be one handler and be registered in the handler table). I am working on those currently as we are doing a MC for the young generation, so I can do the change.

Code-aging: this would be a local modification to the bytecode array.  Is side effect free for parallel / concurrent marking? The update could be made atomic (only one int size write) and could be concurrent to the mutator (it would never be read by the mutator, just reset to zero and read by the GC when visiting the code cache). Would this work with your plans?

Re StaricNewSpaceVisitor, sounds good, thanks.

From a high level view that sounds ok. Note that aging bound to full GCs may age code really fast when we invoke GC often in a short period of time (e.g. in a memory reducing scenario) - which may be OK in that case - or when the heap is small.

Yeah this shouldn't be too much of a problem, since we aren't doing code flushing, so all it will mean is that we quickly evict all code which isn't strongly held (i.e., is from a dead native context). This is probably exactly what we want to do in memory reducing scenarios.

https://codereview.chromium.org/2534763003/ seems to fix the original repo on #0 along with the failing cctest.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/5fd2b71236e59e1cea95f23106de072865bd4857

commit 5fd2b71236e59e1cea95f23106de072865bd4857
Author: rmcilroy <rmcilroy@chromium.org>
Date: Tue Nov 29 12:09:56 2016

[Heap] Remove concept of MarkingParity.

MarkingParity was used to avoid performing an operation on an object if it was
marked multiple times. We no longer mark things multiple times, so this concept
is no longer required.

BUG= chromium:666275 

Review-Url: https://codereview.chromium.org/2529173002
Cr-Commit-Position: refs/heads/master@{#41354}

[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/arm/codegen-arm.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/arm64/codegen-arm64.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/builtins/arm/builtins-arm.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/builtins/arm64/builtins-arm64.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/builtins/builtins.h
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/builtins/ia32/builtins-ia32.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/builtins/mips/builtins-mips.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/builtins/mips64/builtins-mips64.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/builtins/ppc/builtins-ppc.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/builtins/s390/builtins-s390.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/builtins/x64/builtins-x64.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/builtins/x87/builtins-x87.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/heap/mark-compact.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/heap/mark-compact.h
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/heap/objects-visiting-inl.h
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/ia32/codegen-ia32.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/mips/codegen-mips.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/mips/macro-assembler-mips.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/mips64/codegen-mips64.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/mips64/macro-assembler-mips64.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/objects.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/objects.h
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/ppc/codegen-ppc.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/s390/codegen-s390.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/x64/codegen-x64.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/src/x87/codegen-x87.cc
[modify] https://crrev.com/5fd2b71236e59e1cea95f23106de072865bd4857/test/cctest/heap/test-heap.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/067e9e295fa50a3d4a5eb77e6515f30df944069f

commit 067e9e295fa50a3d4a5eb77e6515f30df944069f
Author: rmcilroy <rmcilroy@chromium.org>
Date: Tue Nov 29 12:34:31 2016

[Interpreter] Add bytecode aging and use it enable CompilationCache for bytecode

Adds a bytecode_age field to BytecodeArray objects. This is incremented each
time the bytecode array is marked by GC, and reset to zero if the bytecode
is executed.

This is used to enable the CompilationCache for interpreted functions,
where Interpreted entries are evicted once the bytecode becomes old.

BUG= chromium:666275 , v8:4680 

Review-Url: https://codereview.chromium.org/2534763003
Cr-Commit-Position: refs/heads/master@{#41356}

[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/builtins/arm/builtins-arm.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/builtins/arm64/builtins-arm64.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/builtins/ia32/builtins-ia32.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/builtins/mips/builtins-mips.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/builtins/mips64/builtins-mips64.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/builtins/ppc/builtins-ppc.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/builtins/s390/builtins-s390.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/builtins/x64/builtins-x64.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/builtins/x87/builtins-x87.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/heap/heap.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/heap/objects-visiting-inl.h
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/heap/objects-visiting.h
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/objects-inl.h
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/objects.cc
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/src/objects.h
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/test/cctest/cctest.status
[modify] https://crrev.com/067e9e295fa50a3d4a5eb77e6515f30df944069f/test/cctest/heap/test-heap.cc