looks good overall - couple of questions though.
https://codereview.chromium.org/1023153002/diff/80001/src/heap/gc-idle-time-handler.cc
File src/heap/gc-idle-time-handler.cc (right):
https://codereview.chromium.org/1023153002/diff/80001/src/heap/gc-idle-time-handler.cc#newcode162
src/heap/gc-idle-time-handler.cc:162:
EstimateMarkCompactTime(size_of_objects,
this is looking pretty complex condition now - could you pull each of
these out into seperate well-named variables like mark_compact_rate_high
https://codereview.chromium.org/1023153002/diff/80001/src/heap/gc-idle-time-handler.h
File src/heap/gc-idle-time-handler.h (right):
https://codereview.chromium.org/1023153002/diff/80001/src/heap/gc-idle-time-handler.h#newcode146
src/heap/gc-idle-time-handler.h:146: static const size_t
kMinTimeInBetweenMarkCompactsInMs = 1000;
Is 1000ms a guesstimation or is there any reason to think that pages
shouldn't really need full GCs within 1s?
https://codereview.chromium.org/1023153002/diff/80001/src/heap/heap.cc
File src/heap/heap.cc (right):
https://codereview.chromium.org/1023153002/diff/80001/src/heap/heap.cc#newcode4629
src/heap/heap.cc:4629: heap_state.current_time =
static_cast<size_t>(current_time);
I'm slightly hesitent at setting current_time as a field in heap_state,
because it might lead it to be used at a much later point in time (when
it doesn't represent the actual current time). I realize making this a
separate variable won't change too much but might make this more
obvious, WDYT?
https://codereview.chromium.org/1023153002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
