https://codereview.chromium.org/187683002/diff/130001/src/mark-compact-inl.h
File src/mark-compact-inl.h (right):
https://codereview.chromium.org/187683002/diff/130001/src/mark-compact-inl.h#newcode108
src/mark-compact-inl.h:108: mode == FAIL_ON_OVERFLOW);
This invariant can be expressed in one expression I think.
ASSERT(heap->mark_compact_collector()->IsInsideEvacuationScope() ==
mode != IGNORE_OVERFLOW);
https://codereview.chromium.org/187683002/diff/130001/src/mark-compact-inl.h#newcode132
src/mark-compact-inl.h:132: mode == FAIL_ON_OVERFLOW);
Likewise.
https://codereview.chromium.org/187683002/diff/130001/src/mark-compact.cc
File src/mark-compact.cc (right):
https://codereview.chromium.org/187683002/diff/130001/src/mark-compact.cc#newcode4218
src/mark-compact.cc:4218: EvacuationScope no_evacuation(this);
nit: The naming of this scope (the local variable, not the class) is
weird, shouldn't it be called "in_evacuation" instead, because it
indicates we are evacuating now?
Also, can we move it to the top of the function, to right after the
"gc_scope"?
https://codereview.chromium.org/187683002/diff/130001/src/mark-compact.h
File src/mark-compact.h (right):
https://codereview.chromium.org/187683002/diff/130001/src/mark-compact.h#newcode327
src/mark-compact.h:327:
nit: These two should be made protected IMHO.
https://codereview.chromium.org/187683002/diff/130001/src/mark-compact.h#newcode346
src/mark-compact.h:346: inline void UpdateSlotsRecordedIn(bool
code_slots_filtering_required) {
nit: The naming is a little bit weird now, because it no longer takes an
argument to which the "in" applies. Can we rename the method to
"UpdateRecordedSlots" instead?
https://codereview.chromium.org/187683002/diff/130001/src/mark-compact.h#newcode743
src/mark-compact.h:743: ASSERT(evacuation_scope_ <= 1);
These two ASSERTS combined basically enforce ...
ASSERT(evacuation_scope_ == 1); // in IncrementEvacuationScope
ASSERT(evacuation_scope_ == 0); // in DecrementEvacuationScope
Can we change it to this instead? That would be much clearer.
https://codereview.chromium.org/187683002/diff/130001/src/mark-compact.h#newcode1048
src/mark-compact.h:1048: class EvacuationScope V8_FINAL {
High-level idea: This scope pretty much coincides with "state_ ==
SWEEP_SPACES", would it make sense to use the state variable instead, or
do you think a separate scope brings additional advantages?
https://codereview.chromium.org/187683002/
--
--
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.
