New issue
Advanced search Search tips
Starred by 6 users
Status: Fixed
Owner:
Closed: Aug 21
Cc:
Components:
HW: ----
NextAction: ----
OS: ----
Priority: 2
Type: Bug

Blocking:
issue 6413



Sign in to add a comment
WeakMap.set(obj, value) transitions obj
Reported by stefan.p...@gmail.com, May 16 2017 Back to list
Version: a2304802d8d1349bc7ac280a84b2cffa68b883ea
OS: MacOS/all
Architecture: x64/all

TL;DR `WeakMap.set(obj, value)` transitions `obj` if `obj` has been added for the first time as key to a map/set/weakset/weakmap. This appears to negatively affect map/set/weakset/weakmap performance when objects are used as keys. In addition to affecting collections, it may also cause unexpected deopts of code expecting `obj`'s transitions to have settled.
 
Real Life Use-cases:

Ember.js uses WeakMaps to store meta data about watched objects, as such sites such as travis-ci.org/ twitch.tv, linkedin.com and others tend to spend more time in WeakMap land (and related code) than expected.

What steps will reproduce the problem?

Given:
```js
// index.js
let set = new WeakSet();
let obj = { };
print('-- set.add(obj) --');
set.add(foo);
```

Running:
d8_debug --trace-maps index.js // (d8_debug is just a `make debug` of d8)

What is the expected output?

-- set.add(obj) --
<no transition is expect>

What do you see instead?

-- set.add(obj) --
[TraceMaps: Transition from= 0x3460b3c032d9 to= 0x3460b3c0f569 name= <hash_code_symbol> ]



V8 codez:


codez in v8:

https://github.com/v8/v8/blob/master/src/js/weak-collection.js#L57
https://github.com/v8/v8/blob/9e5a064197ca3db0ff39bcc04767494ead4aa929/src/js/collection.js#L89
https://github.com/v8/v8/blob/9e5a064197ca3db0ff39bcc04767494ead4aa929/src/js/collection.js#L107
https://github.com/v8/v8/blob/9e5a064197ca3db0ff39bcc04767494ead4aa929/src/js/collection.js#L112 <-- mutates obj

Related issues: https://bugs.chromium.org/p/v8/issues/detail?id=3649
Additional words: http://iamstef.net/n/shapeshifting.html

 
Cc: bmeu...@chromium.org adamk@chromium.org cbruni@chromium.org
Components: Runtime
Labels: Priority-2
Owner: gsat...@chromium.org
Status: Assigned
https://docs.google.com/document/d/1qsmgnqXRLa0-vCwdKcgXreCsn9PpajdDjHxx1G25RBY/edit
Project Member Comment 2 by bugdroid1@chromium.org, Jul 11 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/892d49a695615e34d3cfa0a74cfe575e5ef94bc9

commit 892d49a695615e34d3cfa0a74cfe575e5ef94bc9
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Tue Jul 11 22:48:54 2017

[runtime] Introduce PropertyArray

This patch changes the backing store of slow properties to be a
new instance type called PropertyArray.

Currently the only difference between this and a FixedArray is
the map. A future patch will change the length property to store
the hash code.

Bug:  v8:5717 ,  v8:6404 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: Iaebc98f42e6d93c1392772e6f837787beb64afec
Reviewed-on: https://chromium-review.googlesource.com/539028
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46569}
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/bootstrapper.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/code-stub-assembler.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/code-stub-assembler.h
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/compiler/js-create-lowering.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/compiler/js-graph.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/compiler/js-graph.h
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/compiler/types.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/deoptimizer.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/factory.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/factory.h
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/heap/heap.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/heap/heap.h
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/heap/object-stats.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/heap/objects-visiting.h
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/heap/spaces.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/ic/accessor-assembler.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/keys.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/lookup.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/objects-body-descriptors-inl.h
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/objects-debug.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/objects-inl.h
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/objects-printer.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/objects.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/objects.h
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/objects/map.h
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/profiler/heap-snapshot-generator.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/src/runtime/runtime-object.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/test/cctest/test-code-stub-assembler.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/test/cctest/test-elements-kind.cc
[modify] https://crrev.com/892d49a695615e34d3cfa0a74cfe575e5ef94bc9/tools/v8heapconst.py

Project Member Comment 3 by bugdroid1@chromium.org, Jul 13 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/bd910a98684079a3d010cb4679a0a6bd11d41f7b

commit bd910a98684079a3d010cb4679a0a6bd11d41f7b
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Thu Jul 13 21:40:36 2017

[runtime] Rename kPropertiesOffset to kPropertiesOrHashOffset

TBR=bmeurer@chromium.org

Bug:  v8:6404 
Change-Id: Ic813f885449178d10527834356c33da658e2cf06
Reviewed-on: https://chromium-review.googlesource.com/569183
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46652}
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/arm/code-stubs-arm.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/arm/macro-assembler-arm.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/arm64/code-stubs-arm64.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/arm64/macro-assembler-arm64.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/builtins/builtins-arguments-gen.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/builtins/builtins-array-gen.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/builtins/builtins-async-gen.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/builtins/builtins-collections-gen.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/builtins/builtins-constructor-gen.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/builtins/builtins-conversion-gen.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/builtins/builtins-function-gen.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/builtins/builtins-proxy-gen.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/builtins/builtins-string-gen.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/builtins/builtins-typedarray-gen.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/code-stub-assembler.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/compiler/access-builder.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ia32/macro-assembler-ia32.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ic/accessor-assembler.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ic/arm/handler-compiler-arm.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ic/arm64/handler-compiler-arm64.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ic/ia32/handler-compiler-ia32.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ic/mips/handler-compiler-mips.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ic/mips64/handler-compiler-mips64.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ic/ppc/handler-compiler-ppc.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ic/s390/handler-compiler-s390.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ic/x64/handler-compiler-x64.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ic/x87/handler-compiler-x87.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/mips/code-stubs-mips.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/mips/macro-assembler-mips.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/mips64/code-stubs-mips64.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/mips64/macro-assembler-mips64.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/objects-body-descriptors-inl.h
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/objects-inl.h
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/objects.h
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ppc/code-stubs-ppc.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/ppc/macro-assembler-ppc.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/profiler/heap-snapshot-generator.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/s390/code-stubs-s390.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/s390/macro-assembler-s390.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/x64/macro-assembler-x64.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/x87/code-stubs-x87.cc
[modify] https://crrev.com/bd910a98684079a3d010cb4679a0a6bd11d41f7b/src/x87/macro-assembler-x87.cc

Project Member Comment 4 by bugdroid1@chromium.org, Jul 18 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/21a66b11d36f39955c1c331fdf693aa5e9fddfec

commit 21a66b11d36f39955c1c331fdf693aa5e9fddfec
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Tue Jul 18 17:38:50 2017

[runtime] Rename properties to properties_or_hash

Add SetProperties as the generic interface to set properties. In the
future, this will switch based on the input properties type and
correctly store the hash code.

This patch also updates tests to check against empty_property_array
instead of empty_fixed_array.

Bug:  v8:6404 
Change-Id: I39d324ea3ab3cc2c2223b6f4be64139bb88edd94
Reviewed-on: https://chromium-review.googlesource.com/574761
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46744}
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/deoptimizer.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/factory.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/heap/heap.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/lookup.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/objects-debug.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/objects-inl.h
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/objects-printer.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/objects.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/objects.h
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/profiler/heap-snapshot-generator.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/runtime/runtime-literals.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/src/runtime/runtime-object.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/test/cctest/heap/test-heap.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/test/cctest/test-api.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/test/cctest/test-code-stub-assembler.cc
[modify] https://crrev.com/21a66b11d36f39955c1c331fdf693aa5e9fddfec/test/cctest/test-elements-kind.cc

Project Member Comment 5 by bugdroid1@chromium.org, Jul 21 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/b2bf43d5297c679325ed913531dbcb9e7e1164e7

commit b2bf43d5297c679325ed913531dbcb9e7e1164e7
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Fri Jul 21 23:26:53 2017

[runtime] Load only 10 bits as PropertyArray length

Bug:  v8:6404 
Change-Id: I187f20006c14aab4a36e2bfef31ca68ebb249e43
Reviewed-on: https://chromium-review.googlesource.com/576516
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46822}
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/code-stub-assembler.cc
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/code-stub-assembler.h
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/compiler/access-builder.cc
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/compiler/access-builder.h
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/compiler/type-cache.h
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/heap/heap.cc
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/ic/accessor-assembler.cc
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/objects-inl.h
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/objects.h
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/objects/object-macros-undef.h
[modify] https://crrev.com/b2bf43d5297c679325ed913531dbcb9e7e1164e7/src/objects/object-macros.h

Project Member Comment 6 by bugdroid1@chromium.org, Jul 22 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/bb728e182bee7772075b5a9d36db0670171836f9

commit bb728e182bee7772075b5a9d36db0670171836f9
Author: Michael Achenbach <machenbach@chromium.org>
Date: Sat Jul 22 10:46:05 2017

Revert "[runtime] Load only 10 bits as PropertyArray length"

This reverts commit b2bf43d5297c679325ed913531dbcb9e7e1164e7.

Reason for revert: 
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20nosnap%20-%20debug/builds/14149

Original change's description:
> [runtime] Load only 10 bits as PropertyArray length
> 
> Bug:  v8:6404 
> Change-Id: I187f20006c14aab4a36e2bfef31ca68ebb249e43
> Reviewed-on: https://chromium-review.googlesource.com/576516
> Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#46822}

TBR=ulan@chromium.org,jkummerow@chromium.org,mstarzinger@chromium.org,cbruni@chromium.org,gsathya@chromium.org

Change-Id: If55b65f040a5a541726e39c35c12e3a5731aa744
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  v8:6404 
Reviewed-on: https://chromium-review.googlesource.com/582607
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46823}
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/code-stub-assembler.cc
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/code-stub-assembler.h
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/compiler/access-builder.cc
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/compiler/access-builder.h
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/compiler/type-cache.h
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/heap/heap.cc
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/ic/accessor-assembler.cc
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/objects-inl.h
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/objects.h
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/objects/object-macros-undef.h
[modify] https://crrev.com/bb728e182bee7772075b5a9d36db0670171836f9/src/objects/object-macros.h

Project Member Comment 7 by bugdroid1@chromium.org, Jul 24 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/fe18ad65806cc17f669575aeec3defb368bfff6c

commit fe18ad65806cc17f669575aeec3defb368bfff6c
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Mon Jul 24 22:20:06 2017

Reland "[runtime] Load only 10 bits as PropertyArray length"

This is a reland of b2bf43d5297c679325ed913531dbcb9e7e1164e7
Original change's description:
> [runtime] Load only 10 bits as PropertyArray length
> 
> Bug:  v8:6404 
> Change-Id: I187f20006c14aab4a36e2bfef31ca68ebb249e43
> Reviewed-on: https://chromium-review.googlesource.com/576516
> Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#46822}

TBR=ulan@chromium.org,mstarzinger@chromium.org,cbruni@chromium.org

Bug:  v8:6404 
Change-Id: Ia0d8bf276fcfc7bfce704d68ba3427d7ba941ba9
Reviewed-on: https://chromium-review.googlesource.com/583708
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46850}
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/code-stub-assembler.cc
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/code-stub-assembler.h
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/compiler/access-builder.cc
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/compiler/access-builder.h
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/compiler/type-cache.h
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/heap/heap.cc
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/ic/accessor-assembler.cc
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/objects-inl.h
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/objects.h
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/objects/object-macros-undef.h
[modify] https://crrev.com/fe18ad65806cc17f669575aeec3defb368bfff6c/src/objects/object-macros.h

Blocking: 6413
Blocking: chromium:752328
Project Member Comment 10 by bugdroid1@chromium.org, Aug 21
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/01c82f9cab010b390f9ad15de9edf1fee44b972e

commit 01c82f9cab010b390f9ad15de9edf1fee44b972e
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Mon Aug 21 05:30:01 2017

Reland "[runtime] Store hash code in length field"

This is a reland of decf5750c6421adf93b02135defa2b7c6c6fa755

This patch fixes the hash code migration in the backing store
transition case from Smi to PropertyArray in the IC system and
Turbofan. Also, adds tests.

Bug:  v8:6413 ,  v8:6404 

Original change's description:
> [runtime] Store hash code in length field
>
> Store the hash code in 21 bits of the length field.
>
> Change the GetIdentityHash API to be unhandlified, since there's no
> property lookup anymore.
>
> Update js/ and test/ to match new API and expections.
>
> Bug:
> Change-Id: I8dc75de4021f59e79b45f3f38ec997c3b3687b24
> Reviewed-on: https://chromium-review.googlesource.com/589688
> Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47259}

Change-Id: I69289113c4b7978c46f6f9373cc972086ecb6822
Bug: 
Reviewed-on: https://chromium-review.googlesource.com/614903
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47459}
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/builtins/builtins-constructor-gen.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/builtins/builtins-forin-gen.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/builtins/builtins-internal-gen.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/builtins/builtins-proxy-gen.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/code-stub-assembler.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/code-stub-assembler.h
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/compiler/access-builder.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/compiler/access-builder.h
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/compiler/js-builtin-reducer.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/compiler/js-create-lowering.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/compiler/property-access-builder.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/compiler/type-cache.h
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/heap-symbols.h
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/heap/heap.h
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/ic/accessor-assembler.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/ic/keyed-store-generic.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/js/weak-collection.js
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/objects-inl.h
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/objects.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/objects.h
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/objects/dictionary.h
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/runtime/runtime-collections.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/src/runtime/runtime.h
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/test/cctest/BUILD.gn
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/test/cctest/cctest.gyp
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/test/cctest/cctest.h
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/test/cctest/test-api.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/test/cctest/test-dictionary.cc
[add] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/test/cctest/test-hashcode.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/test/cctest/test-inobject-slack-tracking.cc
[modify] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/test/cctest/test-orderedhashtable.cc
[add] https://crrev.com/01c82f9cab010b390f9ad15de9edf1fee44b972e/test/mjsunit/hash-code.js

Status: Fixed
Project Member Comment 12 by bugdroid1@chromium.org, Aug 22
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/942c61ab75b56c22ad8d97c8f61753a16330af67

commit 942c61ab75b56c22ad8d97c8f61753a16330af67
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Tue Aug 22 11:40:09 2017

[runtime] Rename PropertyArray::kLengthOffset to kLengthAndHashOffset

LengthAndHashOffset describes the value stored in the offset better.

Bug:  v8:6404 
Change-Id: Ie5ea2a362c54aa03e0a4e314d1adb8b91d74a044
Reviewed-on: https://chromium-review.googlesource.com/624458
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47503}
[modify] https://crrev.com/942c61ab75b56c22ad8d97c8f61753a16330af67/src/code-stub-assembler.cc
[modify] https://crrev.com/942c61ab75b56c22ad8d97c8f61753a16330af67/src/code-stub-assembler.h
[modify] https://crrev.com/942c61ab75b56c22ad8d97c8f61753a16330af67/src/compiler/access-builder.cc
[modify] https://crrev.com/942c61ab75b56c22ad8d97c8f61753a16330af67/src/compiler/access-builder.h
[modify] https://crrev.com/942c61ab75b56c22ad8d97c8f61753a16330af67/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/942c61ab75b56c22ad8d97c8f61753a16330af67/src/ic/accessor-assembler.cc
[modify] https://crrev.com/942c61ab75b56c22ad8d97c8f61753a16330af67/src/objects-inl.h
[modify] https://crrev.com/942c61ab75b56c22ad8d97c8f61753a16330af67/src/objects.h

Project Member Comment 13 by bugdroid1@chromium.org, Aug 22
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/208cdfd933b7ebf583ddd5e3fdc3eba404abe496

commit 208cdfd933b7ebf583ddd5e3fdc3eba404abe496
Author: Marja Hölttä <marja@chromium.org>
Date: Tue Aug 22 11:59:21 2017

Revert "[runtime] Rename PropertyArray::kLengthOffset to kLengthAndHashOffset"

This reverts commit 942c61ab75b56c22ad8d97c8f61753a16330af67.

Reason for revert: compile failures (mid-air conflict with another cl)

Original change's description:
> [runtime] Rename PropertyArray::kLengthOffset to kLengthAndHashOffset
> 
> LengthAndHashOffset describes the value stored in the offset better.
> 
> Bug:  v8:6404 
> Change-Id: Ie5ea2a362c54aa03e0a4e314d1adb8b91d74a044
> Reviewed-on: https://chromium-review.googlesource.com/624458
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47503}

TBR=jkummerow@chromium.org,mstarzinger@chromium.org,gsathya@chromium.org

Change-Id: I4b439323ab5b328cd8f29908b35eeddffdf5b141
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  v8:6404 
Reviewed-on: https://chromium-review.googlesource.com/626076
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47505}
[modify] https://crrev.com/208cdfd933b7ebf583ddd5e3fdc3eba404abe496/src/code-stub-assembler.cc
[modify] https://crrev.com/208cdfd933b7ebf583ddd5e3fdc3eba404abe496/src/code-stub-assembler.h
[modify] https://crrev.com/208cdfd933b7ebf583ddd5e3fdc3eba404abe496/src/compiler/access-builder.cc
[modify] https://crrev.com/208cdfd933b7ebf583ddd5e3fdc3eba404abe496/src/compiler/access-builder.h
[modify] https://crrev.com/208cdfd933b7ebf583ddd5e3fdc3eba404abe496/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/208cdfd933b7ebf583ddd5e3fdc3eba404abe496/src/ic/accessor-assembler.cc
[modify] https://crrev.com/208cdfd933b7ebf583ddd5e3fdc3eba404abe496/src/objects-inl.h
[modify] https://crrev.com/208cdfd933b7ebf583ddd5e3fdc3eba404abe496/src/objects.h

Project Member Comment 14 by bugdroid1@chromium.org, Aug 22
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/6b915106c6db0ea9dd04e054c1b7d7b4ed3d782e

commit 6b915106c6db0ea9dd04e054c1b7d7b4ed3d782e
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Tue Aug 22 22:34:11 2017

[runtime] Compare against masked_hash while creating identity hash

A random hash could potential have top 10 bits be non zero which would
pass the hash != PropertyArray::kNoHashSentinel test but fail the
masked_hash != PropertyArray::kNoHashSentinel.


Bug:  v8:6404 , chromium:757750
Change-Id: Iade531fefc75dd76bd7a89b377d17e59532087d8
Reviewed-on: https://chromium-review.googlesource.com/627380
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47528}
[modify] https://crrev.com/6b915106c6db0ea9dd04e054c1b7d7b4ed3d782e/src/objects.cc

Project Member Comment 15 by bugdroid1@chromium.org, Aug 23
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/cb3befad02ff21e150558ec94a3e3367f87224de

commit cb3befad02ff21e150558ec94a3e3367f87224de
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Wed Aug 23 04:13:32 2017

Reland "[runtime] Rename PropertyArray::kLengthOffset to kLengthAndHashOffset"

This is a reland of 942c61ab75b56c22ad8d97c8f61753a16330af67
Original change's description:
> [runtime] Rename PropertyArray::kLengthOffset to kLengthAndHashOffset
> 
> LengthAndHashOffset describes the value stored in the offset better.
> 
> Bug:  v8:6404 
> Change-Id: Ie5ea2a362c54aa03e0a4e314d1adb8b91d74a044
> Reviewed-on: https://chromium-review.googlesource.com/624458
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47503}

TBR=jkummerow@chromium.org, mstarzinger@chromium.org

Bug:  v8:6404 
Change-Id: Ied55fa6145ccc788581703db991b2f78d59a7408
Reviewed-on: https://chromium-review.googlesource.com/627075
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47530}
[modify] https://crrev.com/cb3befad02ff21e150558ec94a3e3367f87224de/src/code-stub-assembler.cc
[modify] https://crrev.com/cb3befad02ff21e150558ec94a3e3367f87224de/src/code-stub-assembler.h
[modify] https://crrev.com/cb3befad02ff21e150558ec94a3e3367f87224de/src/compiler/access-builder.cc
[modify] https://crrev.com/cb3befad02ff21e150558ec94a3e3367f87224de/src/compiler/access-builder.h
[modify] https://crrev.com/cb3befad02ff21e150558ec94a3e3367f87224de/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/cb3befad02ff21e150558ec94a3e3367f87224de/src/ic/accessor-assembler.cc
[modify] https://crrev.com/cb3befad02ff21e150558ec94a3e3367f87224de/src/objects-inl.h
[modify] https://crrev.com/cb3befad02ff21e150558ec94a3e3367f87224de/src/objects.h

Blocking: -chromium:752328
Great work!!
Project Member Comment 18 by bugdroid1@chromium.org, Oct 5
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a4bddba0b0dac116d987eea28479dba14663cda0

commit a4bddba0b0dac116d987eea28479dba14663cda0
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Thu Oct 05 19:13:48 2017

[Runtime] Use platform specific value for JSReceiver::HashMask

This allows us to remove the loop while calculating the hash value and
just use the HashMask as the mask for ComputeIntegerHash. This
previously overflowed on 32-bit systems failing the Smi::IsValid
check.

Bug:  v8:6404 
Change-Id: I84610a7592fa9d7ce4fa5cef7903bd50b8e8a4df
Reviewed-on: https://chromium-review.googlesource.com/702675
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48319}
[modify] https://crrev.com/a4bddba0b0dac116d987eea28479dba14663cda0/src/objects.cc
[modify] https://crrev.com/a4bddba0b0dac116d987eea28479dba14663cda0/src/objects.h

Project Member Comment 19 by bugdroid1@chromium.org, Oct 6
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/b7db31a2586bc927fbc63c8d24bd2bb9cbd46ee6

commit b7db31a2586bc927fbc63c8d24bd2bb9cbd46ee6
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Fri Oct 06 15:34:13 2017

[runtime] Templatize PrintFixedArrayElements

Also delete duplicated code

Bug:  v8:6404 
Change-Id: I7f24d99573a854254aa0fd332fe5c947f93e0552
Reviewed-on: https://chromium-review.googlesource.com/704223
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48347}
[modify] https://crrev.com/b7db31a2586bc927fbc63c8d24bd2bb9cbd46ee6/src/objects-printer.cc

Sign in to add a comment