On 2014/03/06 11:13:32, ulan wrote:
Un-overlapped and added VisitCodeNextPointer, PTAL.
https://codereview.chromium.org/181833004/diff/20001/src/objects-visiting-inl.h
File src/objects-visiting-inl.h (right):
https://codereview.chromium.org/181833004/diff/20001/src/objects-visiting-inl.h#newcode936
src/objects-visiting-inl.h:936: !is_optimized_code()) {
On 2014/03/05 08:58:02, titzer wrote:
> On 2014/03/04 12:31:12, Michael Starzinger wrote:
> > As discussed offline: AFAICT, currently we can afford to not have the
fields
> > overlap as the padding is big enough on 32bit and 64bit
architectures. So
> > "un-overlapping" them woukd definitely make things easier to read.
>
> +1 unoverlap. I am sorry for overlapping them in the first place.
Nope, it is my fault. I remember introducing this field and overlapping
it.
Sorry for that.
https://codereview.chromium.org/181833004/diff/20001/src/objects-visiting-inl.h#newcode936
src/objects-visiting-inl.h:936: !is_optimized_code()) {
On 2014/03/04 12:31:12, Michael Starzinger wrote:
> As discussed offline: AFAICT, currently we can afford to not have the
fields
> overlap as the padding is big enough on 32bit and 64bit architectures.
So
> "un-overlapping" them woukd definitely make things easier to read.
Done.
https://codereview.chromium.org/181833004/diff/20001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/181833004/diff/20001/src/objects.h#newcode10690
src/objects.h:10690: enum WeakPointerMode {
On 2014/03/04 12:31:12, Michael Starzinger wrote:
> As discussed offline: I am not a particular fan of having this mode,
because
it
> assumes that all visitors either visit _all_ or _none_ weak pointers
and I
am
> not sure this hold in general.
>
> Alternatively I would suggest having a special visitor method (e.g.
> VisitCodeNextPointer) that the concrete visitor can override to handle
this
slot
> specially. This could be applied to both, the virtual as well as the
static
> visitor I think.
Done. Thanks for suggestion, it is cleaner indeed.
https://codereview.chromium.org/181833004/diff/20001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):
https://codereview.chromium.org/181833004/diff/20001/test/cctest/test-heap.cc#newcode3719
test/cctest/test-heap.cc:3719: TEST(NextCodeLinkIsWeak) {
It assumes two things:
- next_code_link is used for linking
- the optimized code list is a stack (the last optimized code object
points to
the previously optimized code)
Alternative to this would be to manage the list manually. Then I would
have to
- optimize the two functions
- remove the optimized code objects from the context[OPTIMIZED_CODE_LIST]
list
- add two code objects to the list manually.
I agree that this is less brittle in the sense that it wouldn't break if
the
real list management changes. But then there is a risk that the list
management
in this test and the real list management diverge and the test no longer
tests
the implementation.
Agree with your above statement. But it's easy just to have two tests.
https://codereview.chromium.org/181833004/
--
--
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.