I like the change, but I have some objections.
https://codereview.chromium.org/629903003/diff/80001/src/heap/incremental-marking.h
File src/heap/incremental-marking.h (right):
https://codereview.chromium.org/629903003/diff/80001/src/heap/incremental-marking.h#newcode92
src/heap/incremental-marking.h:92: static const size_t
kMaxIdleMarkingDelayCounter = 3;
This is formulated in terms of a number of calls to IdleNotification.
This means a change on the embedder side that inserts extra 0.1ms calls
to IdleNotification can cause the finalization of the GC to happen much
earlier. I think that's unfortunate: I want the scheduler to be able to
say "I found a few moments of time, use them if you can, but don't
perturb your heuristics too much, based on this". Or perhaps "We are
waiting for an important network packet, you can have as many 1ms
snippets as you want until the packet arrives, but we don't want to
delay the processing that is waiting for it when it arrives".
Perhaps this should only count idle notifications that are above a
certain threshold, or perhaps it should add up the ms and use that as
the limit, rather than the invocation count.
https://codereview.chromium.org/629903003/diff/80001/test/unittests/heap/gc-idle-time-handler-unittest.cc
File test/unittests/heap/gc-idle-time-handler-unittest.cc (right):
https://codereview.chromium.org/629903003/diff/80001/test/unittests/heap/gc-idle-time-handler-unittest.cc#newcode179
test/unittests/heap/gc-idle-time-handler-unittest.cc:179: }
I think if you changed kMaxIdleMarkingDelayCounter from 3 to 1000 then
this test would still pass. Can we test that the constant is working as
intended?
https://codereview.chromium.org/629903003/
--
--
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/d/optout.
