I'm going to stop here and ask a more fundamental question: We already have
HashTable from objects.h. Can't we add rehashing-on-GC to that instead of
adding code to the code base?
This implementation increases the number of roots, potentially quite a lot,
thus
making our atomic pauses at the start of GC bigger. HashTable from
objects.h
has the table on-heap so that each n-sized table is just one root instead
of a
bunch of roots.
https://codereview.chromium.org/1105693002/diff/20001/src/heap/heap.cc
File src/heap/heap.cc (right):
https://codereview.chromium.org/1105693002/diff/20001/src/heap/heap.cc#newcode5003
src/heap/heap.cc:5003: for (StrongRootsList* list = strong_roots_list_;
list; list = list->next_) {
It looks like this stuff should be before the serializer/deserializer
iteration and should have its own call to v->Synchronize
https://codereview.chromium.org/1105693002/diff/20001/src/heap/heap.h
File src/heap/heap.h (right):
https://codereview.chromium.org/1105693002/diff/20001/src/heap/heap.h#newcode1478
src/heap/heap.h:1478: delete list;
Might we not just as well return here?
https://codereview.chromium.org/1105693002/diff/20001/src/heap/heap.h#newcode1481
src/heap/heap.h:1481: }
unreachable?
https://codereview.chromium.org/1105693002/diff/20001/src/heap/heap.h#newcode1484
src/heap/heap.h:1484: class OptionalRelocationLock {
Perhaps you could spend a couple of lines of comments here on why we
would want a lock that was optional?
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#newcode41
src/heap/identity-map.cc:41: // xor some of the upper bits, since the
lower 2 or 3 are usually aligned.
Xor
Also see comments below with missing caps and full stops (periods to
you!)
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++) {
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.
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.
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.
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.h
File src/heap/identity-map.h (right):
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.h#newcode68
src/heap/identity-map.h:68: // Note that in general, SMIs are valid
keys, but {SMI #0} is _not_ a valid key.
This is a very strange API. I would either ban SMIs altogether or move
to a tagged null pointer as the internal empty representation and allow
all SMIs.
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.h#newcode86
src/heap/identity-map.h:86: V* Find(Handle<Object> handle) {
Why does this not return a handle?
https://codereview.chromium.org/1105693002/diff/20001/src/heap/identity-map.h#newcode91
src/heap/identity-map.h:91: void Set(Handle<Object> handle, V value) {
handle should be called key? Also several other places?
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.
