incremental-marking-job.* lgtm with one fix needed
Can you add a test using your MockPlatform in cctest/?
https://codereview.chromium.org/1265423002/diff/140001/src/heap/incremental-marking-job.cc
File src/heap/incremental-marking-job.cc (right):
https://codereview.chromium.org/1265423002/diff/140001/src/heap/incremental-marking-job.cc#newcode27
src/heap/incremental-marking-job.cc:27:
made_progress_since_last_delayed_task_ = false;
This should be true, I guess. Otherwise the DelayedTask is forced to do
work all the time (?)
Also, maybe call it NotifyIdleTaskProgress() as it only indicates
progress in idle tasks and not overall (including delayed) tasks.
https://codereview.chromium.org/1265423002/diff/140001/src/heap/incremental-marking-job.cc#newcode45
src/heap/incremental-marking-job.cc:45: const int kDelayInSeconds = 5;
nit: Can we make this a constant in IncrementalMarkingJob?
https://codereview.chromium.org/1265423002/diff/140001/src/heap/incremental-marking-job.h
File src/heap/incremental-marking-job.h (right):
https://codereview.chromium.org/1265423002/diff/140001/src/heap/incremental-marking-job.h#newcode23
src/heap/incremental-marking-job.h:23: // which can happen for
background tab in Chrome.
nit: s/tab/tabs/
https://codereview.chromium.org/1265423002/diff/140001/src/heap/incremental-marking-job.h#newcode62
src/heap/incremental-marking-job.h:62: void Start(Heap* heap) {
IncrementalMarkingJob is reused as far as I see. We should reset the
member fields on start, as we might end up with
made_progress_since_last_delayed_task_ = true from the last round.
(After fixing NotifyProgress())
https://codereview.chromium.org/1265423002/
--
--
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.
