Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(250)

Issue 12217108: Fix code flusher disabling while marking incrementally. (Closed)

Created:
7 years, 10 months ago by Michael Starzinger
Modified:
7 years, 10 months ago
CC:
v8-dev
Visibility:
Public.

Description

Fix code flusher disabling while marking incrementally. This fixes a corner case where the code flusher is disabled while the incremental marker is still running. This can happen when the debugger is loaded and a scavenge is triggered. Make sure that all flushing decisions are revisited after the candidates lists are evicted. R=hpayer@chromium.org BUG=chromium:173458, chromium:168582 TEST=cctest/test-heap/Regress173458 Committed: http://code.google.com/p/v8/source/detail?r=13641

Patch Set 1 #

Patch Set 2 : Added regression test. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -8 lines) Patch
M src/mark-compact.cc View 2 chunks +4 lines, -8 lines 0 comments Download
M test/cctest/test-heap.cc View 1 1 chunk +51 lines, -0 lines 2 comments Download

Messages

Total messages: 5 (0 generated)
Michael Starzinger
7 years, 10 months ago (2013-02-11 14:19:22 UTC) #1
Hannes Payer (out of office)
LGTM! A regression test would be nice.
7 years, 10 months ago (2013-02-11 14:21:34 UTC) #2
Michael Starzinger
Done. Uploaded a regression test.
7 years, 10 months ago (2013-02-11 14:47:11 UTC) #3
Hannes Payer (out of office)
https://codereview.chromium.org/12217108/diff/4001/test/cctest/test-heap.cc File test/cctest/test-heap.cc (right): https://codereview.chromium.org/12217108/diff/4001/test/cctest/test-heap.cc#newcode2877 test/cctest/test-heap.cc:2877: } In the future, we may wanna have that ...
7 years, 10 months ago (2013-02-11 14:48:58 UTC) #4
Michael Starzinger
7 years, 10 months ago (2013-02-11 15:01:33 UTC) #5
https://codereview.chromium.org/12217108/diff/4001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

https://codereview.chromium.org/12217108/diff/4001/test/cctest/test-heap.cc#n...
test/cctest/test-heap.cc:2877: }
On 2013/02/11 14:48:58, Hannes Payer wrote:
> In the future, we may wanna have that code in a function
SimulateCodeFlushing().

Agreed, I'll clean that up in a follow-up CL. But let's not do it in this one,
because we want to be able to merge this one back to the branches easily.

Powered by Google App Engine
This is Rietveld 408576698