Maintain a FixedArray for the optimized code map.

Maintain a FixedArray for the optimized code map.

This simplifies follow-on changes to the FastNewClosureStub.

BUG=

Committed: https://crrev.com/f7b6e3815c3eb3f33bb08f2a939b95c7bde2d31a
Cr-Commit-Position: refs/heads/master@{#32123}

[mvstanton, 2015-11-11 09:02:56]

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

[mvstanton, 2015-11-11 09:02:56]

Hi Michael,
This eliminates the smi case for the optimized code map, using the root as we
discussed. Perhaps this should be landed after branch?
Thanks,
--Michael

[Yang, 2015-11-11 10:44:33]

On 2015/11/11 09:02:56, mvstanton wrote:
> Hi Michael,
> This eliminates the smi case for the optimized code map, using the root as we
> discussed. Perhaps this should be landed after branch?
> Thanks,
> --Michael

src/debug lgtm.

[mvstanton, 2015-11-12 13:56:53]

Hi Michael, you can look at this one when you like. Thanks,
--Michael

[Michael Starzinger, 2015-11-13 12:14:22]

LGTM. Only some nits and suggestions to address.

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

https://codereview.chromium.org/1433923002/diff/40001/src/flag-definitions.h#...
src/flag-definitions.h:383: DEFINE_BOOL(flush_optimized_code_cache, false,
Yep, as discussed offline: Let's land this CL after the branch and after we made
this flag switch in a separate CL. The flag switch should be kept in a separate
CL for easy rollback.

https://codereview.chromium.org/1433923002/diff/40001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1433923002/diff/40001/src/heap/heap.cc#newcod...
src/heap/heap.cc:2792: DCHECK(SharedFunctionInfo::kEntriesStart == 1 &&
nit: Could be turned into a STATIC_ASSERT instead.

https://codereview.chromium.org/1433923002/diff/40001/src/heap/heap.cc#newcod...
src/heap/heap.cc:2794:
cleared_optimized_code_map->set(SharedFunctionInfo::kSharedCodeIndex,
The field is already initialized to undefined, because Factory::NewFixedArray
produces an initialed array. Explicitly setting this field is not needed.

https://codereview.chromium.org/1433923002/diff/40001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1433923002/diff/40001/src/objects.cc#newcode1...
src/objects.cc:11136: if (optimized_code_map() != cleared_map) {
suggestion: Not sure this optimization is worth it. I would vote for using the
OptimizedCodeMapIsCleared() predicate here as well for readability. Decision is
up to you.

https://codereview.chromium.org/1433923002/diff/40001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1433923002/diff/40001/src/objects.h#newcode6561
src/objects.h:6561: inline bool OptimizedCodeMapIsCleared() const;
nit: Needs a comment explaining the nature of the "empty optimized code map".
Also move up the predicate to right after ClearOptimizedCodeMap to make it clear
that these things belong together.

[mvstanton, 2015-11-17 20:32:12]

Thanks, comments addressed, much appreciated!
--Mike

https://codereview.chromium.org/1433923002/diff/40001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1433923002/diff/40001/src/heap/heap.cc#newcod...
src/heap/heap.cc:2792: DCHECK(SharedFunctionInfo::kEntriesStart == 1 &&
On 2015/11/13 12:14:22, Michael Starzinger wrote:
> nit: Could be turned into a STATIC_ASSERT instead.

Done.

https://codereview.chromium.org/1433923002/diff/40001/src/heap/heap.cc#newcod...
src/heap/heap.cc:2794:
cleared_optimized_code_map->set(SharedFunctionInfo::kSharedCodeIndex,
On 2015/11/13 12:14:22, Michael Starzinger wrote:
> The field is already initialized to undefined, because Factory::NewFixedArray
> produces an initialed array. Explicitly setting this field is not needed.

Done.

https://codereview.chromium.org/1433923002/diff/40001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1433923002/diff/40001/src/objects.cc#newcode1...
src/objects.cc:11136: if (optimized_code_map() != cleared_map) {
On 2015/11/13 12:14:22, Michael Starzinger wrote:
> suggestion: Not sure this optimization is worth it. I would vote for using the
> OptimizedCodeMapIsCleared() predicate here as well for readability. Decision
is
> up to you.

Done.

https://codereview.chromium.org/1433923002/diff/40001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1433923002/diff/40001/src/objects.h#newcode6561
src/objects.h:6561: inline bool OptimizedCodeMapIsCleared() const;
On 2015/11/13 12:14:22, Michael Starzinger wrote:
> nit: Needs a comment explaining the nature of the "empty optimized code map".
> Also move up the predicate to right after ClearOptimizedCodeMap to make it
clear
> that these things belong together.

Done.

[mvstanton, 2015-11-19 17:29:11]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2015-11-19 17:29:11]

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

[commit-bot: I haz the power, 2015-11-19 17:29:23]

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

[commit-bot: I haz the power, 2015-11-19 19:31:20]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-11-19 19:32:08]

Patchset 5 (id:??) landed as
https://crrev.com/f7b6e3815c3eb3f33bb08f2a939b95c7bde2d31a
Cr-Commit-Position: refs/heads/master@{#32123}