Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1577263003:
[heap, deoptimizer] Use proper right trim instead of manually trimming (Closed)
|
Created:
4 years, 11 months ago by Michael Lippautz Modified:
4 years, 11 months ago Reviewers:
Hannes Payer (out of office) CC:
v8-reviews_googlegroups.com, Hannes Payer (out of office), ulan Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[heap, deoptimizer] Use proper right trim instead of manually trimming
Failing to do so results in out-of-date marking information, because live bytes
is not properly adjusted.
This CL adds support for right trimming ByteArray and properly DCHECKs that we
do not left trim ByteArray (as we already do for FixedTypedArrayBase).
BUG=
Committed: https://crrev.com/8cf798736f0922b337bf8a62497239a89712cc0b
Cr-Commit-Position: refs/heads/master@{#33252}
Patch Set 1 : #
Total comments: 4
Patch Set 2 : Addressed comments #
Total comments: 2
Patch Set 3 : Undo DCHECK for delta, as it can become 0 #Patch Set 4 : rebase #
Messages
Total messages: 25 (15 generated)
Description was changed from ========== [heap/deoptimizier] Use proper RightTrim instead of manually trimming. Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. BUG= ========== to ========== [heap/deoptimizier] Use proper RightTrim instead of manually trimming. Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already to for FixedTypedArrayBase). BUG= ==========
Patchset #1 (id:1) has been deleted
Description was changed from ========== [heap/deoptimizier] Use proper RightTrim instead of manually trimming. Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already to for FixedTypedArrayBase). BUG= ========== to ========== [heap/deoptimizier] Use proper right trim instead of manually trimming. Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already to for FixedTypedArrayBase). BUG= ==========
Description was changed from ========== [heap/deoptimizier] Use proper right trim instead of manually trimming. Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already to for FixedTypedArrayBase). BUG= ========== to ========== [heap/deoptimizier] Use proper right trim instead of manually trimming Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already to for FixedTypedArrayBase). BUG= ==========
Patchset #1 (id:20001) has been deleted
mlippautz@chromium.org changed reviewers: + hpayer@chromium.org
PTAL
Description was changed from ========== [heap/deoptimizier] Use proper right trim instead of manually trimming Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already to for FixedTypedArrayBase). BUG= ========== to ========== [heap/deoptimizier] Use proper right trim instead of manually trimming Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already do for FixedTypedArrayBase). BUG= ==========
https://codereview.chromium.org/1577263003/diff/40001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/1577263003/diff/40001/src/heap/heap.cc#newcod... src/heap/heap.cc:3191: } else { else if https://codereview.chromium.org/1577263003/diff/40001/src/ia32/deoptimizer-ia... File src/ia32/deoptimizer-ia32.cc (right): https://codereview.chromium.org/1577263003/diff/40001/src/ia32/deoptimizer-ia... src/ia32/deoptimizer-ia32.cc:165: if (delta > 0) { Shouldn't the delta here always be larger than 0? DCHECK instead?
https://codereview.chromium.org/1577263003/diff/40001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/1577263003/diff/40001/src/heap/heap.cc#newcod... src/heap/heap.cc:3191: } else { On 2016/01/12 14:57:12, Hannes Payer wrote: > else if Done. https://codereview.chromium.org/1577263003/diff/40001/src/ia32/deoptimizer-ia... File src/ia32/deoptimizer-ia32.cc (right): https://codereview.chromium.org/1577263003/diff/40001/src/ia32/deoptimizer-ia... src/ia32/deoptimizer-ia32.cc:165: if (delta > 0) { On 2016/01/12 14:57:12, Hannes Payer wrote: > Shouldn't the delta here always be larger than 0? DCHECK instead? Done. I also added a DCHECK_GE() in RightTrimFixedArray. https://codereview.chromium.org/1577263003/diff/60001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/1577263003/diff/60001/src/heap/heap.cc#newcod... src/heap/heap.cc:3183: DCHECK_LE(elements_to_trim, len); Note that this changed from < to <=, as the deoptimizer sometimes fully trims a bytearray, leaving an array with len==0 behind.
lgtm https://codereview.chromium.org/1577263003/diff/60001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/1577263003/diff/60001/src/heap/heap.cc#newcod... src/heap/heap.cc:3183: DCHECK_LE(elements_to_trim, len); On 2016/01/12 15:52:54, Michael Lippautz wrote: > Note that this changed from < to <=, as the deoptimizer sometimes fully trims a > bytearray, leaving an array with len==0 behind. Acknowledged.
The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1577263003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1577263003/60001
Description was changed from ========== [heap/deoptimizier] Use proper right trim instead of manually trimming Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already do for FixedTypedArrayBase). BUG= ========== to ========== [heap, deoptimizer] Use proper right trim instead of manually trimming Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already do for FixedTypedArrayBase). BUG= ==========
The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1577263003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1577263003/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)
The CQ bit was checked by mlippautz@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/1577263003/#ps100001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1577263003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1577263003/100001
Message was sent while issue was closed.
Description was changed from ========== [heap, deoptimizer] Use proper right trim instead of manually trimming Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already do for FixedTypedArrayBase). BUG= ========== to ========== [heap, deoptimizer] Use proper right trim instead of manually trimming Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already do for FixedTypedArrayBase). BUG= ==========
Message was sent while issue was closed.
Committed patchset #4 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== [heap, deoptimizer] Use proper right trim instead of manually trimming Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already do for FixedTypedArrayBase). BUG= ========== to ========== [heap, deoptimizer] Use proper right trim instead of manually trimming Failing to do so results in out-of-date marking information, because live bytes is not properly adjusted. This CL adds support for right trimming ByteArray and properly DCHECKs that we do not left trim ByteArray (as we already do for FixedTypedArrayBase). BUG= Committed: https://crrev.com/8cf798736f0922b337bf8a62497239a89712cc0b Cr-Commit-Position: refs/heads/master@{#33252} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/8cf798736f0922b337bf8a62497239a89712cc0b Cr-Commit-Position: refs/heads/master@{#33252} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
