[v8.x] deps: cherry-pick a803fad from upstream V8 #19824
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 <[email protected]>
Commit-Queue: Sathya Gunasekaran <[email protected]>
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 <[email protected]>
Commit-Queue: Camillo Bruni <[email protected]>
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 <[email protected]>
Commit-Queue: Benedikt Meurer <[email protected]>
Cr-Commit-Position: refs/heads/master@{nodejs#48480}
Refs: v8/v8@a803fad
nodejs-github-bot
added
v8 engine
|
/cc @nodejs/v8-update |
hashseed
approved these changes
Apr 5, 2018
|
V8 CI was green, |
gibfahn
approved these changes
Apr 12, 2018
gibfahn
pushed a commit
that referenced
this pull request
Apr 12, 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 <[email protected]>
Commit-Queue: Sathya Gunasekaran <[email protected]>
Cr-Commit-Position: refs/heads/master@{#48319}
PR-URL: #19824
Refs: v8/v8@a4bddba
Fixes: #19769
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
gibfahn
pushed a commit
that referenced
this pull request
Apr 12, 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 <[email protected]>
Commit-Queue: Camillo Bruni <[email protected]>
Cr-Commit-Position: refs/heads/master@{#47804}
PR-URL: #19824
Refs: v8/v8@7abdadc
Fixes: #19769
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
gibfahn
pushed a commit
that referenced
this pull request
Apr 12, 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 <[email protected]>
Commit-Queue: Benedikt Meurer <[email protected]>
Cr-Commit-Position: refs/heads/master@{#48480}
PR-URL: #19824
Refs: v8/v8@a803fad
Fixes: #19769
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
|
Landed in 53b702f...f12db24 Thanks @targos ! |
Merged
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.
Same as #19770, with version numbers changed.
Fixes: #19769
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes