Use WeakCells in the optimized code map rather than traversing in pause.

Use WeakCells in the optimized code map rather than traversing in pause.

It's expensive to walk all shared function infos during the gc atomic pause. Instead, use WeakCells to implement this structure without manual clearing.

BUG=

Committed: https://crrev.com/20a8162f081215706310ea3c943dbd69ac670aeb
Cr-Commit-Position: refs/heads/master@{#32567}

[mvstanton, 2015-11-26 14:52:42]

Patchset #2 (id:20001) has been deleted

[mvstanton, 2015-11-26 14:52:49]

Patchset #1 (id:1) has been deleted

[mvstanton, 2015-11-26 14:54:19]

Description was changed from

==========
Use WeakCells in the optimized code map rather than traversing in pause.

BUG=
==========

to

==========
Use WeakCells in the optimized code map rather than traversing in pause.

It's expensive to walk all shared function infos during the gc atomic pause.
Instead, use WeakCells to implement this structure without manual clearing.

BUG=
==========

[mvstanton, 2015-11-26 14:54:19]

mvstanton@chromium.org changed reviewers:
+ mstarzinger@chromium.org, ulan@chromium.org

[mvstanton, 2015-11-26 14:54:19]

mvstanton@chromium.org changed required reviewers:
+ mstarzinger@chromium.org, ulan@chromium.org

[mvstanton, 2015-11-26 14:54:47]

Hi Ulan, hi Michael,
PTAL, thanks!
--Mike S.

[ulan, 2015-11-27 10:03:18]

Looking good!

https://codereview.chromium.org/1478943003/diff/60001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1478943003/diff/60001/src/objects.cc#newcode1...
src/objects.cc:11440: SKIP_WRITE_BARRIER);
Either
1) keep the write barrier
2) or add empty_weak_cell to IMMORTAL_IMMOVABLE_ROOT_LIST

https://codereview.chromium.org/1478943003/diff/60001/src/objects.cc#newcode1...
src/objects.cc:11460: // Copy old optimized code map and append one new entry.
Should we try to compact the array before growing it?

[Michael Starzinger, 2015-11-27 10:06:40]

https://codereview.chromium.org/1478943003/diff/60001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/1478943003/diff/60001/src/flag-definitions.h#...
src/flag-definitions.h:387: DEFINE_BOOL(flush_optimized_code_cache, false,
I assume this is only for the try-jobs and we want to land the flag flip in a
separate CL. Correct?

https://codereview.chromium.org/1478943003/diff/60001/src/heap/gc-tracer.cc
File src/heap/gc-tracer.cc (right):

https://codereview.chromium.org/1478943003/diff/60001/src/heap/gc-tracer.cc#n...
src/heap/gc-tracer.cc:508: "mark_optimizedcodemaps=%.1f "
Better remove this line from the format string, otherwise numbers are completely
mixed up.

https://codereview.chromium.org/1478943003/diff/60001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (left):

https://codereview.chromium.org/1478943003/diff/60001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:2166: void
MarkCompactCollector::ProcessAndClearOptimizedCodeMaps() {
Woot, I love it!

https://codereview.chromium.org/1478943003/diff/60001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1478943003/diff/60001/src/objects-inl.h#newco...
src/objects-inl.h:2039: DCHECK(GetHeap()->gc_state() == Heap::MARK_COMPACT ||
value()->IsUndefined());
I assume this exception is just here because of the call to "clear" in
Heap::CreateInitialObjects. Correct? Could we instead check "this ==
GetHeap()->empty_weak_cell()" to make that clearer and keep the filter of what
is allowed narrow?

[mvstanton, 2015-12-01 11:28:16]

Hi Michi, hi Ulan,
Comments addressed, thanks!
--Mike

https://codereview.chromium.org/1478943003/diff/60001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/1478943003/diff/60001/src/flag-definitions.h#...
src/flag-definitions.h:387: DEFINE_BOOL(flush_optimized_code_cache, false,
On 2015/11/27 10:06:39, Michael Starzinger wrote:
> I assume this is only for the try-jobs and we want to land the flag flip in a
> separate CL. Correct?

Correct! Setting it back to true :).

https://codereview.chromium.org/1478943003/diff/60001/src/heap/gc-tracer.cc
File src/heap/gc-tracer.cc (right):

https://codereview.chromium.org/1478943003/diff/60001/src/heap/gc-tracer.cc#n...
src/heap/gc-tracer.cc:508: "mark_optimizedcodemaps=%.1f "
On 2015/11/27 10:06:39, Michael Starzinger wrote:
> Better remove this line from the format string, otherwise numbers are
completely
> mixed up.

whoa, thanks!

https://codereview.chromium.org/1478943003/diff/60001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (left):

https://codereview.chromium.org/1478943003/diff/60001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:2166: void
MarkCompactCollector::ProcessAndClearOptimizedCodeMaps() {
On 2015/11/27 10:06:39, Michael Starzinger wrote:
> Woot, I love it!

Acknowledged.

https://codereview.chromium.org/1478943003/diff/60001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1478943003/diff/60001/src/objects-inl.h#newco...
src/objects-inl.h:2039: DCHECK(GetHeap()->gc_state() == Heap::MARK_COMPACT ||
value()->IsUndefined());
On 2015/11/27 10:06:40, Michael Starzinger wrote:
> I assume this exception is just here because of the call to "clear" in
> Heap::CreateInitialObjects. Correct? Could we instead check "this ==
> GetHeap()->empty_weak_cell()" to make that clearer and keep the filter of what
> is allowed narrow?

Good idea, done.

https://codereview.chromium.org/1478943003/diff/60001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1478943003/diff/60001/src/objects.cc#newcode1...
src/objects.cc:11440: SKIP_WRITE_BARRIER);
On 2015/11/27 10:03:18, ulan wrote:
> Either
> 1) keep the write barrier
> 2) or add empty_weak_cell to IMMORTAL_IMMOVABLE_ROOT_LIST

Nice! I'll do 2), because I'd like to believe I can skip!

https://codereview.chromium.org/1478943003/diff/60001/src/objects.cc#newcode1...
src/objects.cc:11460: // Copy old optimized code map and append one new entry.
On 2015/11/27 10:03:18, ulan wrote:
> Should we try to compact the array before growing it?

I'll do a search for a hole. If there is a cleared native context, then we can
use that entry for our new addition.

[Michael Starzinger, 2015-12-01 12:36:10]

LGTM.

https://codereview.chromium.org/1478943003/diff/120001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1478943003/diff/120001/src/objects.cc#newcode...
src/objects.cc:11462: DCHECK(entry < kEntriesStart);
nit: DCHECK_LT

[ulan, 2015-12-01 12:53:08]

LGTM

[mvstanton, 2015-12-03 14:19:53]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2015-12-03 14:19:53]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org,
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1478943003/#ps140001
(title: "REBASE.")

[commit-bot: I haz the power, 2015-12-03 14:20:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1478943003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1478943003/140001

[commit-bot: I haz the power, 2015-12-03 14:57:22]

Description was changed from

==========
Use WeakCells in the optimized code map rather than traversing in pause.

It's expensive to walk all shared function infos during the gc atomic pause.
Instead, use WeakCells to implement this structure without manual clearing.

BUG=
==========

to

==========
Use WeakCells in the optimized code map rather than traversing in pause.

It's expensive to walk all shared function infos during the gc atomic pause.
Instead, use WeakCells to implement this structure without manual clearing.

BUG=
==========

[commit-bot: I haz the power, 2015-12-03 14:57:22]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2015-12-03 14:57:57]

Description was changed from

==========
Use WeakCells in the optimized code map rather than traversing in pause.

It's expensive to walk all shared function infos during the gc atomic pause.
Instead, use WeakCells to implement this structure without manual clearing.

BUG=
==========

to

==========
Use WeakCells in the optimized code map rather than traversing in pause.

It's expensive to walk all shared function infos during the gc atomic pause.
Instead, use WeakCells to implement this structure without manual clearing.

BUG=

Committed: https://crrev.com/20a8162f081215706310ea3c943dbd69ac670aeb
Cr-Commit-Position: refs/heads/master@{#32567}
==========

[commit-bot: I haz the power, 2015-12-03 14:57:58]

Patchset 6 (id:??) landed as
https://crrev.com/20a8162f081215706310ea3c943dbd69ac670aeb
Cr-Commit-Position: refs/heads/master@{#32567}