Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1441633002:
Optimize MarkCompactCollector::ClearNonLiveReferences. (Closed)
|
Created:
5 years, 1 month ago by ulan Modified:
5 years, 1 month ago Reviewers:
Hannes Payer (out of office) CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionOptimize MarkCompactCollector::ClearNonLiveReferences.
This splits the work to be done for dead and life maps
and avoids unnecessary operations in the hot loop.
Results for v8.infinite_scroll:
Before:
nonlive_refs
len: 93
min: 0.0
max: 6.3
avg: 1.35268817204
[0,5[: 91
[5,10[: 2
After:
nonlive_refs
len: 91
min: 0.0
max: 4.2
avg: 0.968131868132
[0,5[: 91
BUG=chromium:554488
LOG=no
Committed: https://crrev.com/ed357f9696f36a64f8457aa99c0269d6cdcc1ec4
Cr-Commit-Position: refs/heads/master@{#32042}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Address comment #Patch Set 3 : #Patch Set 4 : Fix assert #Patch Set 5 : Correctly fix assertion #Messages
Total messages: 24 (12 generated)
ulan@chromium.org changed reviewers: + hpayer@chromium.org
PTAL https://codereview.chromium.org/1441633002/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1441633002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:2205: ClearNonLivePrototypeTransitions(map); We don't need to compact prototype transitions for dead maps because the array will not be used anymore and weak cells make sure that there are no dangling pointers. https://codereview.chromium.org/1441633002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:2207: ClearNonLiveMapTransitions(map); We don't need to call this function for live maps because it does meaningful work only for (live parent) -> (dead map) transitions.
lgtm https://codereview.chromium.org/1441633002/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1441633002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:2203: bool alive = !Marking::IsWhite(map_mark); Let's ask Marking::IsBlackOrGrey here for liveness, it is more direct. https://codereview.chromium.org/1441633002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:2205: ClearNonLivePrototypeTransitions(map); On 2015/11/12 12:00:37, ulan wrote: > We don't need to compact prototype transitions for dead maps because the array > will not be used anymore and weak cells make sure that there are no dangling > pointers. Awesome! https://codereview.chromium.org/1441633002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:2207: ClearNonLiveMapTransitions(map); On 2015/11/12 12:00:37, ulan wrote: > We don't need to call this function for live maps because it does meaningful > work only for (live parent) -> (dead map) transitions. Awesome^2!
https://codereview.chromium.org/1441633002/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1441633002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:2203: bool alive = !Marking::IsWhite(map_mark); On 2015/11/12 19:35:59, Hannes Payer wrote: > Let's ask Marking::IsBlackOrGrey here for liveness, it is more direct. Done.
The CQ bit was checked by ulan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org Link to the patchset: https://codereview.chromium.org/1441633002/#ps20001 (title: "Address comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1441633002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1441633002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux_arm_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/10203) v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...) v8_linux_mipsel_compile_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...) v8_mac_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/11915)
The CQ bit was checked by ulan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org Link to the patchset: https://codereview.chromium.org/1441633002/#ps40001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1441633002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1441633002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux_dbg on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg/builds/10823)
The CQ bit was checked by ulan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org Link to the patchset: https://codereview.chromium.org/1441633002/#ps60001 (title: "Fix assert")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1441633002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1441633002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_mac_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/11923)
The CQ bit was checked by ulan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org Link to the patchset: https://codereview.chromium.org/1441633002/#ps80001 (title: "Correctly fix assertion")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1441633002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1441633002/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/ed357f9696f36a64f8457aa99c0269d6cdcc1ec4 Cr-Commit-Position: refs/heads/master@{#32042} |
