Fix for 3303 MultithreadedParallelIsolates has a race condition.

Fix for 3303 MultithreadedParallelIsolates has a race condition.

The fix is to make the code aging sequence hang off the isolate.

BUG=v8:3303
R=svenpanne@chromium.org
LOG=N

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

[mvstanton, 2014-05-02 14:58:35]

Hi Sven,
Here are with changes and ports. Thx for the look!
--Michael

[Sven Panne, 2014-05-05 07:54:13]

https://codereview.chromium.org/261953002/diff/100001/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

https://codereview.chromium.org/261953002/diff/100001/src/arm/simulator-arm.c...
src/arm/simulator-arm.cc:637: if (i_cache == NULL) return;
Huh? Does this really belong to this CL?

https://codereview.chromium.org/261953002/diff/100001/src/arm64/macro-assembl...
File src/arm64/macro-assembler-arm64.cc (right):

https://codereview.chromium.org/261953002/diff/100001/src/arm64/macro-assembl...
src/arm64/macro-assembler-arm64.cc:5100: // TODO(mvstanton): this static isn't
thread safe.
Perhaps NoCodeAgeSequence should have a more generic name and be used here, too?

https://codereview.chromium.org/261953002/diff/100001/src/codegen.h
File src/codegen.h (right):

https://codereview.chromium.org/261953002/diff/100001/src/codegen.h#newcode128
src/codegen.h:128: bool Match(byte* candidate) const {
"IsPrefixOf" describes the intent more closely.

[mvstanton, 2014-05-06 09:55:45]

Hi Sven, thanks for the comments, PTAL,
--Michael

https://codereview.chromium.org/261953002/diff/100001/src/arm64/macro-assembl...
File src/arm64/macro-assembler-arm64.cc (right):

https://codereview.chromium.org/261953002/diff/100001/src/arm64/macro-assembl...
src/arm64/macro-assembler-arm64.cc:5100: // TODO(mvstanton): this static isn't
thread safe.
On 2014/05/05 07:54:14, Sven Panne wrote:
> Perhaps NoCodeAgeSequence should have a more generic name and be used here,
too?

I changed it to CodeAgingHelper, and it can determine if your code is old or
young, this addressed the thread safety issue too.

https://codereview.chromium.org/261953002/diff/100001/src/codegen.h
File src/codegen.h (right):

https://codereview.chromium.org/261953002/diff/100001/src/codegen.h#newcode128
src/codegen.h:128: bool Match(byte* candidate) const {
On 2014/05/05 07:54:14, Sven Panne wrote:
> "IsPrefixOf" describes the intent more closely.

I ended up generalizing the class, and the notion of "IsYoung()" and "IsOld()"
worked nicely.

https://codereview.chromium.org/261953002/diff/120001/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

https://codereview.chromium.org/261953002/diff/120001/src/arm/simulator-arm.c...
src/arm/simulator-arm.cc:637: if (i_cache == NULL) return;
The reason this condition was being hit at snapshot time, is that now we create
instructions on isolate initialization, rather than lazily when we want to test
code for young/old status. In the ARM simulator environment, caching isn't set
up until later in the initialization process.

A better fix is that when the CodePatcher is initialized to create the
CodeAgingHelper::young_sequence_, we can ask that code flushing not be used.

[Sven Panne, 2014-05-06 10:03:09]

lgtm

[mvstanton, 2014-05-06 11:25:53]

Committed patchset #6 manually as r21165 (presubmit successful).