LGTM
https://codereview.chromium.org/640303006/diff/180001/src/heap/heap.cc
File src/heap/heap.cc (right):
https://codereview.chromium.org/640303006/diff/180001/src/heap/heap.cc#newcode2702
src/heap/heap.cc:2702:
WeakCell::cast(result)->update_value_from_gc(value);
This is from allocation, not from GC, so probably should not call this
function.
https://codereview.chromium.org/640303006/diff/180001/src/heap/heap.cc#newcode2703
src/heap/heap.cc:2703:
WeakCell::cast(result)->set_next(undefined_value());
There's no need to set the write barrier when setting to undefined, is
there? I think it's immovable and immortal. So we could add
SKIP_WRITE_BARRIER here.
https://codereview.chromium.org/640303006/diff/180001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):
https://codereview.chromium.org/640303006/diff/180001/src/heap/mark-compact.cc#newcode2748
src/heap/mark-compact.cc:2748: weak_cell->set_next(undefined);
No write barrier needed.
https://codereview.chromium.org/640303006/diff/180001/src/heap/mark-compact.cc#newcode2760
src/heap/mark-compact.cc:2760: weak_cell->set_next(undefined);
No write barrier needed
https://codereview.chromium.org/640303006/diff/180001/src/heap/mark-compact.cc#newcode2777
src/heap/mark-compact.cc:2777: // We scavange new space simultaneously
with sweeping. This is done in two
scavange -> scavenge
https://codereview.chromium.org/640303006/diff/180001/src/objects-inl.h
File src/objects-inl.h (right):
https://codereview.chromium.org/640303006/diff/180001/src/objects-inl.h#newcode1940
src/objects-inl.h:1940: WRITE_BARRIER(GetHeap(), this, kValueOffset,
val);
If we are in GC, do we need the write barrier? I think this is only
legitimately used to set the value to 'undefined', so pehaps it should
be renamed clear_value_from_gc and it should not take an argument.
https://codereview.chromium.org/640303006/diff/180001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):
https://codereview.chromium.org/640303006/diff/180001/test/cctest/test-heap.cc#newcode4262
test/cctest/test-heap.cc:4262: heap->CollectAllAvailableGarbage();
The tests should also test that the weak cell is unchanged if there is a
full GC and the value is also reachable through normal objects.
https://codereview.chromium.org/640303006/
--
--
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.
