[Interpreter] Avoid dereferencing handles in ConstantPoolArrayBuilder.

[Interpreter] Avoid dereferencing handles in ConstantPoolArrayBuilder.

Changes ConstantPoolArrayBuilder to do object lookups using the location
of the handles, rather than dereferencing the handles and comparing the
objects. This also updates CanonicalHandleScope when internalizing AST
nodes to ensure that duplicate objects share the same handles and so are
only added to the constant pool once.

BUG=v8:5203
Committed: https://crrev.com/297f2d831a569cf253374ac36841b1e1cb70f601
Cr-Commit-Position: refs/heads/master@{#38366}

[rmcilroy, 2016-08-03 22:44:23]

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

[commit-bot: I haz the power, 2016-08-03 22:44:27]

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, 2016-08-03 22:56:29]

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

[commit-bot: I haz the power, 2016-08-03 22:56:29]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)

[rmcilroy, 2016-08-04 11:01:27]

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

[commit-bot: I haz the power, 2016-08-04 11:01:34]

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, 2016-08-04 11:22:28]

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

[commit-bot: I haz the power, 2016-08-04 11:22:29]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2016-08-04 11:36:18]

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

[rmcilroy, 2016-08-04 11:36:19]

Michi, ptal, thanks.

[rmcilroy, 2016-08-04 21:49:53]

Patchset #2 (id:20001) has been deleted

[rmcilroy, 2016-08-04 21:50:02]

Patchset #1 (id:1) has been deleted

[rmcilroy, 2016-08-04 21:50:12]

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

[commit-bot: I haz the power, 2016-08-04 21:50:17]

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, 2016-08-04 22:10:38]

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

[commit-bot: I haz the power, 2016-08-04 22:10:39]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2016-08-05 09:19:06]

LGTM.

https://codereview.chromium.org/2204243003/diff/40001/test/unittests/interpre...
File test/unittests/interpreter/bytecode-array-builder-unittest.cc (right):

https://codereview.chromium.org/2204243003/diff/40001/test/unittests/interpre...
test/unittests/interpreter/bytecode-array-builder-unittest.cc:25:
BytecodeArrayBuilder builder(isolate(), zone(), 0, 1, 131);
nit: IIUC then we are just lucky here because no constant is used twice. Can we
add a CanonicalHandleScope just for safety?

https://codereview.chromium.org/2204243003/diff/40001/test/unittests/interpre...
File test/unittests/interpreter/constant-array-builder-unittest.cc (right):

https://codereview.chromium.org/2204243003/diff/40001/test/unittests/interpre...
test/unittests/interpreter/constant-array-builder-unittest.cc:182:
TEST_F(ConstantArrayBuilderTest, GapFilledWhenLowReservationCommitted) {
nit: Can we add a CanonicalHandleScope here as well, even though it's not
needed? Just for consistency to avoid confusion when this test is used as a
template for other tests.

[rmcilroy, 2016-08-05 09:45:08]

https://codereview.chromium.org/2204243003/diff/40001/test/unittests/interpre...
File test/unittests/interpreter/bytecode-array-builder-unittest.cc (right):

https://codereview.chromium.org/2204243003/diff/40001/test/unittests/interpre...
test/unittests/interpreter/bytecode-array-builder-unittest.cc:25:
BytecodeArrayBuilder builder(isolate(), zone(), 0, 1, 131);
On 2016/08/05 09:19:06, Michael Starzinger wrote:
> nit: IIUC then we are just lucky here because no constant is used twice. Can
we
> add a CanonicalHandleScope just for safety?

Added to all tests in this file.

https://codereview.chromium.org/2204243003/diff/40001/test/unittests/interpre...
File test/unittests/interpreter/constant-array-builder-unittest.cc (right):

https://codereview.chromium.org/2204243003/diff/40001/test/unittests/interpre...
test/unittests/interpreter/constant-array-builder-unittest.cc:182:
TEST_F(ConstantArrayBuilderTest, GapFilledWhenLowReservationCommitted) {
On 2016/08/05 09:19:06, Michael Starzinger wrote:
> nit: Can we add a CanonicalHandleScope here as well, even though it's not
> needed? Just for consistency to avoid confusion when this test is used as a
> template for other tests.

Opps missed this one. Done.

[rmcilroy, 2016-08-05 09:45:53]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-08-05 09:45:54]

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/2204243003/#ps60001
(title: "Address comments")

[commit-bot: I haz the power, 2016-08-05 09:45:59]

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, 2016-08-05 10:09:54]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-08-05 10:10:08]

Description was changed from

==========
[Interpreter] Avoid dereferencing handles in ConstantPoolArrayBuilder.

Changes ConstantPoolArrayBuilder to do object lookups using the location
of the handles, rather than dereferencing the handles and comparing the
objects. This also updates CanonicalHandleScope when internalizing AST
nodes to ensure that duplicate objects share the same handles and so are
only added to the constant pool once.

BUG=v8:5203
==========

to

==========
[Interpreter] Avoid dereferencing handles in ConstantPoolArrayBuilder.

Changes ConstantPoolArrayBuilder to do object lookups using the location
of the handles, rather than dereferencing the handles and comparing the
objects. This also updates CanonicalHandleScope when internalizing AST
nodes to ensure that duplicate objects share the same handles and so are
only added to the constant pool once.

BUG=v8:5203

Committed: https://crrev.com/297f2d831a569cf253374ac36841b1e1cb70f601
Cr-Commit-Position: refs/heads/master@{#38366}
==========

[commit-bot: I haz the power, 2016-08-05 10:10:09]

Patchset 2 (id:??) landed as
https://crrev.com/297f2d831a569cf253374ac36841b1e1cb70f601
Cr-Commit-Position: refs/heads/master@{#38366}