This is already looking good, I like this change. Just a few comments about
the
mechanics of the implementation.
https://codereview.chromium.org/16641003/diff/3001/src/heap.cc
File src/heap.cc (right):
https://codereview.chromium.org/16641003/diff/3001/src/heap.cc#newcode1499
src/heap.cc:1499: Heap* heap,
nit: Indentation is off, also put the "record_slots" parameter on its
own line.
https://codereview.chromium.org/16641003/diff/3001/src/heap.cc#newcode1569
src/heap.cc:1569: context->set_unchecked(heap,
I think we can finally switch to use the checked accessor here, this
means 'context' will always be a valid heap-object and we don't need to
thread the Heap pointer through set_weak_next.
This is a leftover from the old GC implementation that used in-place
marking and destroyed the map word in object headers. It is tracked by
issue 1490.
https://codereview.chromium.org/16641003/diff/3001/src/heap.cc#newcode1633
src/heap.cc:1633: static void set_weak_next(Heap*, JSTypedArray* obj,
Object* next) {
See my previous comment, I think we can drop the Heap pointer argument
here and for the other visitors.
https://codereview.chromium.org/16641003/diff/3001/src/heap.cc#newcode1642
src/heap.cc:1642: Heap* heap,
nit: Can we make the Heap pointer be the first argument to the function?
This would be in line with the other visitors in the code-base.
https://codereview.chromium.org/16641003/
--
--
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/groups/opt_out.
