[v9.x] deps: cherry-pick a803fad from upstream V8 #19770
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original commit message:
[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@{nodejs#48319}
Refs: v8/v8@a4bddba
Original commit message:
Sprinkle some DisallowHeapAllocation
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I7d34ccddeea08f5935e360e8c36791365f27f89e
Reviewed-on: https://chromium-review.googlesource.com/647706
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{nodejs#47804}
Refs: v8/v8@7abdadc
Original commit message:
Make sure the identity hash is uniform (at least in the lower bits).
In the current implementation of hash code for objects (identity hash),
we do not bother to shift the hash when we retrieve it from the
hash-length bitfield in a property array. (Even worse, we store shifted
value even if we do not have property array or inside dictionaries.)
That means that the hash-code for objects is always divisible by 1024.
Since our hash table uses a simple masking with (2^logsize - 1) to
obtain the bucket, we get terrible hash collisions - essentially, our
hash table degenerates to a linked list for fewer than 1024 elements.
This CL always shifts the hash code so that the value in the lowest
21 bits is uniformly distributed.
This results in big improvements on medium to large hash tables.
A program storing 1M elements into a WeakMap gets roughly
17x faster. A program retrieving 1M elements from a Map
improves even more dramatically (>100x).
const a = [];
for (let i = 0; i < 1e6; i++) a[i] = {};
const m = new Map();
console.time("Map.set");
for (let i = 0; i < 1e6; i++) {
m.set(a[i], i);
}
console.timeEnd("Map.set");
console.time("Map.get");
let s = 0;
for (let i = 0; i < 1e6; i++) {
s += m.get(a[i]);
}
console.timeEnd("Map.get");
const w = new WeakMap();
console.time("WeakMap.set");
for (let i = 0; i < 1e6; i++) {
w.set(a[i], i);
}
console.timeEnd("WeakMap.set");
Before the fix:
Map.set: 157.575000
Map.get: 28333.182000
WeakMap.set: 6923.826000
After the fix:
Map.set: 178.382000
Map.get: 185.930000
WeakMap.set: 409.529000
Note that Map does not suffer from the hash collision on insertion because
it uses chaining (insertion into linked list is fast regardless of size!), and
we cleverly avoid lookup in the hash table on update if the key does not have
identity hash yet. This is in contrast to the WeakMap, which uses
open-addressing, and deals with collisions on insertion.
Bug: v8:6916
Change-Id: Ic5497bd4501e3b767b3f4acb7efb4784cbb3a2e4
Reviewed-on: https://chromium-review.googlesource.com/713616
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{nodejs#48480}
Refs: v8/v8@a803fad
nodejs-github-bot
added
build
targos
added
v8 engine
ofrobots
approved these changes
Apr 3, 2018
|
@targos can you please use @nodejs/v8-update (instead of @nodejs/v8) for anything related to updating v8? Thanks |
|
@gsathya I chose to ping the whole team this time because it's not "just" an update. I'd like to make sure chances are high that an expert from the V8 team takes a look at the changes. I should probably have made that clear in the OP. |
2 tasks
targos
added a commit
that referenced
this pull request
Apr 5, 2018
Original commit message:
[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}
Refs: v8/v8@a4bddba
PR-URL: #19770
Fixes: #19769
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
targos
added a commit
that referenced
this pull request
Apr 5, 2018
Original commit message:
Sprinkle some DisallowHeapAllocation
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I7d34ccddeea08f5935e360e8c36791365f27f89e
Reviewed-on: https://chromium-review.googlesource.com/647706
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47804}
Refs: v8/v8@7abdadc
PR-URL: #19770
Fixes: #19769
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
targos
added a commit
that referenced
this pull request
Apr 5, 2018
Original commit message:
Make sure the identity hash is uniform (at least in the lower bits).
In the current implementation of hash code for objects (identity hash),
we do not bother to shift the hash when we retrieve it from the
hash-length bitfield in a property array. (Even worse, we store shifted
value even if we do not have property array or inside dictionaries.)
That means that the hash-code for objects is always divisible by 1024.
Since our hash table uses a simple masking with (2^logsize - 1) to
obtain the bucket, we get terrible hash collisions - essentially, our
hash table degenerates to a linked list for fewer than 1024 elements.
This CL always shifts the hash code so that the value in the lowest
21 bits is uniformly distributed.
This results in big improvements on medium to large hash tables.
A program storing 1M elements into a WeakMap gets roughly
17x faster. A program retrieving 1M elements from a Map
improves even more dramatically (>100x).
const a = [];
for (let i = 0; i < 1e6; i++) a[i] = {};
const m = new Map();
console.time("Map.set");
for (let i = 0; i < 1e6; i++) {
m.set(a[i], i);
}
console.timeEnd("Map.set");
console.time("Map.get");
let s = 0;
for (let i = 0; i < 1e6; i++) {
s += m.get(a[i]);
}
console.timeEnd("Map.get");
const w = new WeakMap();
console.time("WeakMap.set");
for (let i = 0; i < 1e6; i++) {
w.set(a[i], i);
}
console.timeEnd("WeakMap.set");
Before the fix:
Map.set: 157.575000
Map.get: 28333.182000
WeakMap.set: 6923.826000
After the fix:
Map.set: 178.382000
Map.get: 185.930000
WeakMap.set: 409.529000
Note that Map does not suffer from the hash collision on insertion because
it uses chaining (insertion into linked list is fast regardless of size!), and
we cleverly avoid lookup in the hash table on update if the key does not have
identity hash yet. This is in contrast to the WeakMap, which uses
open-addressing, and deals with collisions on insertion.
Bug: v8:6916
Change-Id: Ic5497bd4501e3b767b3f4acb7efb4784cbb3a2e4
Reviewed-on: https://chromium-review.googlesource.com/713616
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48480}
Refs: v8/v8@a803fad
PR-URL: #19770
Fixes: #19769
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This also includes two other commits to avoid cherry-pick conflicts.
Fixes: #19769
Commits in v8/v8:
v8/v8@a4bddba
v8/v8@7abdadc
v8/v8@a803fad
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes