Tighten the interface to the optimized code map

Tighten the interface to the optimized code map

We either want to add code+literals to the map, or just literals.
A recent change in the structure of the map (it now uses WeakCells)
meant that we have to be more clear about what we want to do the right
thing.

BUG=

Committed: https://crrev.com/909f93d052e561d6196e4b6e083b46923591d46b
Cr-Commit-Position: refs/heads/master@{#32761}

[mvstanton, 2015-12-10 14:51:08]

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

[mvstanton, 2015-12-10 14:51:08]

Hi Ulan, hi Michael,
Here is the patch we discussed, thx for the look!
--Mike

[Michael Starzinger, 2015-12-10 14:57:29]

https://codereview.chromium.org/1516833002/diff/20001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1516833002/diff/20001/src/objects.h#newcode6518
src/objects.h:6518: inline static void AddLiteralsToOptimizedCodeMap(
nit: Can we have a short comment explaining this method? Just so that the
difference to the above one is documented.

https://codereview.chromium.org/1516833002/diff/20001/src/objects.h#newcode6520
src/objects.h:6520: Handle<LiteralsArray> literals, BailoutId osr_ast_id);
This should not need a {BailoutId}, because without code this value doesn't make
much sense. Can we remove that argument from this interface?

[mvstanton, 2015-12-10 15:21:49]

Thx for comments, PTAL :).
--Mike

https://codereview.chromium.org/1516833002/diff/20001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1516833002/diff/20001/src/objects.h#newcode6518
src/objects.h:6518: inline static void AddLiteralsToOptimizedCodeMap(
On 2015/12/10 14:57:28, Michael Starzinger wrote:
> nit: Can we have a short comment explaining this method? Just so that the
> difference to the above one is documented.

Done.

https://codereview.chromium.org/1516833002/diff/20001/src/objects.h#newcode6520
src/objects.h:6520: Handle<LiteralsArray> literals, BailoutId osr_ast_id);
On 2015/12/10 14:57:28, Michael Starzinger wrote:
> This should not need a {BailoutId}, because without code this value doesn't
make
> much sense. Can we remove that argument from this interface?

doh! Absolutely, thanks!

[Michael Starzinger, 2015-12-10 15:32:10]

LGTM with one nit to address.

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

https://codereview.chromium.org/1516833002/diff/60001/src/objects.h#newcode6526
src/objects.h:6526: static void
AddToOptimizedCodeMapInternal(Handle<SharedFunctionInfo> shared,
nit: There already is "SearchOptimizedCodeMapEntry" in the private section
further down below. Please move this down close to there instead of having this
private section in the middle. Thanks!

[ulan, 2015-12-10 15:39:32]

LGTM

[mvstanton, 2015-12-10 15:42:43]

Thanks guys :)

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

https://codereview.chromium.org/1516833002/diff/60001/src/objects.h#newcode6526
src/objects.h:6526: static void
AddToOptimizedCodeMapInternal(Handle<SharedFunctionInfo> shared,
On 2015/12/10 15:32:09, Michael Starzinger wrote:
> nit: There already is "SearchOptimizedCodeMapEntry" in the private section
> further down below. Please move this down close to there instead of having
this
> private section in the middle. Thanks!

Good point, done.

[mvstanton, 2015-12-10 15:42:51]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2015-12-10 15:42:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1516833002/#ps80001
(title: "Moved definition to private section.")

[commit-bot: I haz the power, 2015-12-10 15:43:02]

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

[Michael Starzinger, 2015-12-10 15:43:48]

https://codereview.chromium.org/1516833002/diff/80001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1516833002/diff/80001/src/objects.h#newcode6524
src/objects.h:6524: public:
nit: Please also drop this re-public. :)

[commit-bot: I haz the power, 2015-12-10 17:03:26]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-12-10 17:03:52]

Description was changed from

==========
Tighten the interface to the optimized code map

We either want to add code+literals to the map, or just literals.
A recent change in the structure of the map (it now uses WeakCells)
meant that we have to be more clear about what we want to do the right
thing.

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

to

==========
Tighten the interface to the optimized code map

We either want to add code+literals to the map, or just literals.
A recent change in the structure of the map (it now uses WeakCells)
meant that we have to be more clear about what we want to do the right
thing.

BUG=

Committed: https://crrev.com/909f93d052e561d6196e4b6e083b46923591d46b
Cr-Commit-Position: refs/heads/master@{#32761}
==========

[commit-bot: I haz the power, 2015-12-10 17:03:53]

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

[mvstanton, 2015-12-10 18:09:33]

https://codereview.chromium.org/1516833002/diff/80001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1516833002/diff/80001/src/objects.h#newcode6524
src/objects.h:6524: public:
On 2015/12/10 15:43:48, Michael Starzinger wrote:
> nit: Please also drop this re-public. :)

<sheepish> New CL (https://codereview.chromium.org/1516003002/)