On 2013/06/11 12:59:53, Michael Starzinger wrote:
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.
Excellent, I am happy that Heap parameter can now be removed! Will do.
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.
