https://codereview.chromium.org/1105693002/diff/1/src/heap/identity-map.cc
File src/heap/identity-map.cc (right):
https://codereview.chromium.org/1105693002/diff/1/src/heap/identity-map.cc#newcode50
src/heap/identity-map.cc:50: Resize(); // Should only have to resize
once, since we grow 4x.
On 2015/04/27 15:06:32, Hannes Payer wrote:
Let's assert that we just resize once.
Resize() will fail if it gets too big. Should I count the number of
resizes here?
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.cc
File src/heap/identity-map.cc (right):
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.cc#newcode40
src/heap/identity-map.cc:40: CHECK_NE(0, raw_address); // cannot store
Smi 0 as a key here, sorry.
On 2015/04/27 15:06:32, Hannes Payer wrote:
Cannot
Done.
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.cc#newcode41
src/heap/identity-map.cc:41: // xor some of the upper bits, since the
lower 2 or 3 are usually aligned.
On 2015/04/27 15:39:13, Erik Corry wrote:
Xor
Also see comments below with missing caps and full stops (periods to
you!)
Done.
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.cc#newcode64
src/heap/identity-map.cc:64: for (int index = start; index < size_;
index++) {
On 2015/04/27 15:39:13, Erik Corry wrote:
For hashes that are near the end of the array we will resize
immediately (by
4x!), rather than wrap around and search a bit more.
Good catch. I wrap the search now.
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.cc#newcode70
src/heap/identity-map.cc:70: if (--limit == 0) break; // searched too
far; resize to insert.
On 2015/04/27 15:06:32, Hannes Payer wrote:
Searched
Done.
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.cc#newcode72
src/heap/identity-map.cc:72: Resize(); // Should only have to resize
once, since we grow 4x.
On 2015/04/27 15:39:13, Erik Corry wrote:
That's very aggressive. Was 2x too slow?
Also this heuristic for growing will make the timing of the growing
dependent on
ASLR, which is rather unpredictable for my taste.
4x guarantees that there are no chains that size_ / 2 the next time
through the loop, because now size_ is 4x as big.
The backstory here is that we expect these tables to be sparse and
rather than go for a complex hashing scheme, start with one that's much
easier to get right. Thus I wanted to avoid doing buckets and chains
which would require a more complex interface to the GC.
I considered doing cuckoo hashing (e.g. after a collision, use some more
bits from the hash for the next search location). Without any data about
the collision frequency, the complexity doesn't seem justified yet.
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.cc#newcode129
src/heap/identity-map.cc:129: void IdentityMapBase::Rehash() {
On 2015/04/27 15:06:32, Hannes Payer wrote:
Can we assert here that we have the relocation lock?
Unfortunately that API appears to have been refactored away...
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.cc#newcode167
src/heap/identity-map.cc:167: size_ = size_ * 4;
On 2015/04/27 15:06:32, Hannes Payer wrote:
4 = kResizeFactor; make this a constant
Done.
https://codereview.chromium.org/1105693002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
