Unify implementations of Map handles vectors and lists

Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By: franzih@chromium.org
BUG=v8:6333, v8:6325
LOG=N

Review-Url: https://codereview.chromium.org/2809923002
Cr-Commit-Position: refs/heads/master@{#45211}
Committed: https://chromium.googlesource.com/v8/v8/+/fc274fbe99e5c254bb4568d497dd4b8e6e7433e9

[danno, 2017-04-28 09:24:21]

Description was changed from

==========
Unify MapHandleVector/List implementation
==========

to

==========
Unify MapHandleVector/List implementation

Also-By:franzih@chromium.org
==========

[danno, 2017-04-30 07:11:53]

Description was changed from

==========
Unify MapHandleVector/List implementation

Also-By:franzih@chromium.org
==========

to

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By:franzih@chromium.org
==========

[danno, 2017-04-30 07:15:28]

Description was changed from

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By:franzih@chromium.org
==========

to

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By:franzih@chromium.org
BUG=V8:6333
LOG=N
==========

[danno, 2017-04-30 07:15:38]

Description was changed from

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By:franzih@chromium.org
BUG=V8:6333
LOG=N
==========

to

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By:franzih@chromium.org
BUG=v8:6333
LOG=N
==========

[danno, 2017-04-30 07:15:54]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-30 07:16:00]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danno, 2017-04-30 07:16:26]

Description was changed from

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By:franzih@chromium.org
BUG=v8:6333
LOG=N
==========

to

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By=franzih@chromium.org
BUG=v8:6333
LOG=N
==========

[danno, 2017-04-30 07:16:54]

Description was changed from

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By=franzih@chromium.org
BUG=v8:6333
LOG=N
==========

to

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By: franzih@chromium.org
BUG=v8:6333
LOG=N
==========

[danno, 2017-04-30 07:17:46]

Description was changed from

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By: franzih@chromium.org
BUG=v8:6333
LOG=N
==========

to

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By: franzih@chromium.org
BUG=v8:6333,v8:6325
LOG=N
==========

[commit-bot: I haz the power, 2017-04-30 07:47:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-30 07:47:47]

Dry run: This issue passed the CQ dry run.

[danno, 2017-05-02 13:45:37]

danno@chromium.org changed reviewers:
+ franzih@chromium.org

[danno, 2017-05-02 13:45:40]

PTAL

[Franzi, 2017-05-02 16:29:32]

LGTM with nit. Thanks!

https://codereview.chromium.org/2809923002/diff/120001/src/compiler/access-in...
File src/compiler/access-info.cc (right):

https://codereview.chromium.org/2809923002/diff/120001/src/compiler/access-in...
src/compiler/access-info.cc:539: for (auto map : maps) {
Can we make map const?

https://codereview.chromium.org/2809923002/diff/120001/src/ic/ic.cc
File src/ic/ic.cc (right):

https://codereview.chromium.org/2809923002/diff/120001/src/ic/ic.cc#newcode2114
src/ic/ic.cc:2114: while (current_map < last_valid_map) {
Are there strong performance reasons for this pattern? If not, I'd prefer a
remove_if() followed by an erase().

[danno, 2017-05-03 15:09:40]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 15:09:49]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danno, 2017-05-03 15:10:29]

danno@chromium.org changed reviewers:
+ jkummerow@chromium.org

[danno, 2017-05-03 15:10:29]

danno@chromium.org changed reviewers:
+ jkummerow@chromium.org

[danno, 2017-05-03 15:10:30]

+jkummerow for ic directory review

https://codereview.chromium.org/2809923002/diff/120001/src/compiler/access-in...
File src/compiler/access-info.cc (right):

https://codereview.chromium.org/2809923002/diff/120001/src/compiler/access-in...
src/compiler/access-info.cc:539: for (auto map : maps) {
On 2017/05/02 16:29:32, Franzi wrote:
> Can we make map const?

Instead I changed all the autos to Handle<Map>. This matches the original code.
I didn't make them const, since it doesn't add any value here (you can't assign
to Handle<Map>s anyway).

https://codereview.chromium.org/2809923002/diff/120001/src/ic/ic.cc
File src/ic/ic.cc (right):

https://codereview.chromium.org/2809923002/diff/120001/src/ic/ic.cc#newcode2114
src/ic/ic.cc:2114: while (current_map < last_valid_map) {
On 2017/05/02 16:29:32, Franzi wrote:
> Are there strong performance reasons for this pattern? If not, I'd prefer a
> remove_if() followed by an erase(). 

Done.

[danno, 2017-05-03 15:10:30]

+jkummerow for ic directory review

https://codereview.chromium.org/2809923002/diff/120001/src/compiler/access-in...
File src/compiler/access-info.cc (right):

https://codereview.chromium.org/2809923002/diff/120001/src/compiler/access-in...
src/compiler/access-info.cc:539: for (auto map : maps) {
On 2017/05/02 16:29:32, Franzi wrote:
> Can we make map const?

Instead I changed all the autos to Handle<Map>. This matches the original code.
I didn't make them const, since it doesn't add any value here (you can't assign
to Handle<Map>s anyway).

https://codereview.chromium.org/2809923002/diff/120001/src/ic/ic.cc
File src/ic/ic.cc (right):

https://codereview.chromium.org/2809923002/diff/120001/src/ic/ic.cc#newcode2114
src/ic/ic.cc:2114: while (current_map < last_valid_map) {
On 2017/05/02 16:29:32, Franzi wrote:
> Are there strong performance reasons for this pattern? If not, I'd prefer a
> remove_if() followed by an erase(). 

Done.

[commit-bot: I haz the power, 2017-05-03 15:40:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-03 15:40:15]

Dry run: This issue passed the CQ dry run.

[danno, 2017-05-03 15:51:44]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 15:51:57]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-03 16:30:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-03 16:31:00]

Dry run: This issue passed the CQ dry run.

[Jakob Kummerow, 2017-05-05 15:28:39]

LGTM with nits.

https://codereview.chromium.org/2809923002/diff/160001/src/ic/ic.cc
File src/ic/ic.cc (left):

https://codereview.chromium.org/2809923002/diff/160001/src/ic/ic.cc#oldcode1429
src/ic/ic.cc:1429: // Filter out deprecated maps to ensure their instances get
migrated.
nit: this comment got lost

https://codereview.chromium.org/2809923002/diff/160001/src/ic/ic.cc#oldcode2112
src/ic/ic.cc:2112: // Filter out deprecated maps to ensure their instances get
migrated.
nit: this comment got lost

[danno, 2017-05-09 21:29:03]

Feedback addressed, landing.

https://codereview.chromium.org/2809923002/diff/160001/src/ic/ic.cc
File src/ic/ic.cc (left):

https://codereview.chromium.org/2809923002/diff/160001/src/ic/ic.cc#oldcode1429
src/ic/ic.cc:1429: // Filter out deprecated maps to ensure their instances get
migrated.
On 2017/05/05 15:28:39, Jakob Kummerow wrote:
> nit: this comment got lost

Done.

https://codereview.chromium.org/2809923002/diff/160001/src/ic/ic.cc#oldcode2112
src/ic/ic.cc:2112: // Filter out deprecated maps to ensure their instances get
migrated.
On 2017/05/05 15:28:39, Jakob Kummerow wrote:
> nit: this comment got lost

Done.

[danno, 2017-05-09 21:29:09]

The CQ bit was checked by danno@chromium.org

[danno, 2017-05-09 21:29:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
franzih@chromium.org, jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/2809923002/#ps180001
(title: "Review feedback")

[commit-bot: I haz the power, 2017-05-09 21:29:20]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 22:36:01]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1494365349104960,
"parent_rev": "a459f188fab8e026cc7f19bb5eacd01b12699da1", "commit_rev":
"fc274fbe99e5c254bb4568d497dd4b8e6e7433e9"}

[commit-bot: I haz the power, 2017-05-09 22:36:10]

Description was changed from

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By: franzih@chromium.org
BUG=v8:6333,v8:6325
LOG=N
==========

to

==========
Unify implementations of Map handles vectors and lists

This CL introduces a new type, MapHandles, which is a STL vector of Handle<Map>.
It is now used everywhere where lists of Handle<Maps> are required, replacing
usages of V8's internal List type.

Also-By: franzih@chromium.org
BUG=v8:6333,v8:6325
LOG=N

Review-Url: https://codereview.chromium.org/2809923002
Cr-Commit-Position: refs/heads/master@{#45211}
Committed:
https://chromium.googlesource.com/v8/v8/+/fc274fbe99e5c254bb4568d497dd4b8e6e7...
==========

[commit-bot: I haz the power, 2017-05-09 22:36:11]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/v8/v8/+/fc274fbe99e5c254bb4568d497dd4b8e6e7...