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

Issue 1197713004: Fix terrible interaction with code flushing. (Closed)

Created:
5 years, 6 months ago by Michael Starzinger
Modified:
5 years, 6 months ago
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Fix terrible interaction with code flushing. This fixes a terrible interaction of code flushing and the clearing of optimized code maps hanging off a SharedFunctionInfo. The following is what happened: 1) Incremental marking cleared map in SharedFunctionInfo s, however it was not enqueued as a flushing candidate because one JSFunction f1 still had optimized code. 2) Deoptimization of f1 made s eligible for code flushing. 3) Optimization of f2 added new entry to optimized code map of s. 4) The JSFunction f2 became unreachable and hence is never marked. 5) Incremental marking now visits f1, finds it eligible for flushing, also s is eligible for flushing, both are enqueued. 6) Marking finishes, code flusher clears f1 and s, but the optimized code map of s still contains an entry. 7) Boom! R=ulan@chromium.org,hpayer@chromium.org TEST=mjsunit/es6/generators-iteration BUG=v8:3803 LOG=N Committed: https://crrev.com/816abc5e864e503a5c9802483f555c339d7ac52f Cr-Commit-Position: refs/heads/master@{#29177}

Patch Set 1 #

Patch Set 2 : Small cleanup. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -4 lines) Patch
M src/compiler.cc View 1 1 chunk +4 lines, -1 line 0 comments Download
M src/factory.cc View 1 chunk +1 line, -0 lines 0 comments Download
M src/heap/mark-compact.cc View 2 chunks +10 lines, -0 lines 0 comments Download
M test/mjsunit/mjsunit.status View 1 chunk +0 lines, -3 lines 0 comments Download

Messages

Total messages: 8 (1 generated)
Michael Starzinger
5 years, 6 months ago (2015-06-19 15:24:09 UTC) #1
Hannes Payer (out of office)
lgtm
5 years, 6 months ago (2015-06-19 19:21:28 UTC) #2
ulan
On 2015/06/19 15:24:09, Michael Starzinger wrote: lgtm!
5 years, 6 months ago (2015-06-22 07:43:07 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1197713004/20001
5 years, 6 months ago (2015-06-22 08:03:55 UTC) #5
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 6 months ago (2015-06-22 08:25:38 UTC) #6
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/816abc5e864e503a5c9802483f555c339d7ac52f Cr-Commit-Position: refs/heads/master@{#29177}
5 years, 6 months ago (2015-06-22 08:25:58 UTC) #7
wingo
5 years, 6 months ago (2015-06-22 09:06:33 UTC) #8
Message was sent while issue was closed.
Michael you are amazing.  Thank you!!

Powered by Google App Engine
This is Rietveld 408576698