Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2443923002:
[interpreter] Ensure --debug-code works with snapshots. (Closed)
|
Created:
4 years, 1 month ago by Michael Starzinger Modified:
4 years, 1 month ago Reviewers:
rmcilroy CC:
v8-reviews_googlegroups.com, rmcilroy Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[interpreter] Ensure --debug-code works with snapshots.
This makes sure that bytecode handlers are regenerated when debugging
code within handlers is being requested. We cannot use the handlers
baked into the snapshot in this case.
R=rmcilroy@chromium.org
Committed: https://crrev.com/438c5eb28bb9a20782c03cd70dcdf3fa04f09e28
Cr-Commit-Position: refs/heads/master@{#40555}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Addressed comment. #
Messages
Total messages: 19 (13 generated)
The CQ bit was checked by mstarzinger@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [interpreter] Ensure --debug-code works with snapshots. This makes sure that bytecode handlers are regenerated when debugging code within handlers is being requested. We cannot use then handlers baked into the snapshot in this case. R=rmcilroy@chromium.org ========== to ========== [interpreter] Ensure --debug-code works with snapshots. This makes sure that bytecode handlers are regenerated when debugging code within handlers is being requested. We cannot use the handlers baked into the snapshot in this case. R=rmcilroy@chromium.org ==========
LGTM. Optional request while you are here. https://codereview.chromium.org/2443923002/diff/1/src/interpreter/interpreter.cc File src/interpreter/interpreter.cc (right): https://codereview.chromium.org/2443923002/diff/1/src/interpreter/interpreter... src/interpreter/interpreter.cc:217: FLAG_trace_ignition_dispatches || FLAG_debug_code) { While you are here, there is a bug that GetBytecodeHandler calls IsDispatchTableInitialized which means any calls to GetBytecodeHandler fail if any of these flags are set. Could you create a seperate function (ShouldInitializeDispatchTable) which checks these flags, and have IsDispatchTableInitialized only check dispatch_table_[0] != nullptr ?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/15124) v8_linux_dbg_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)
The CQ bit was checked by mstarzinger@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Comments addressed. https://codereview.chromium.org/2443923002/diff/1/src/interpreter/interpreter.cc File src/interpreter/interpreter.cc (right): https://codereview.chromium.org/2443923002/diff/1/src/interpreter/interpreter... src/interpreter/interpreter.cc:217: FLAG_trace_ignition_dispatches || FLAG_debug_code) { On 2016/10/24 13:46:54, rmcilroy wrote: > While you are here, there is a bug that GetBytecodeHandler calls > IsDispatchTableInitialized which means any calls to GetBytecodeHandler fail if > any of these flags are set. Could you create a seperate function > (ShouldInitializeDispatchTable) which checks these flags, and have > IsDispatchTableInitialized only check dispatch_table_[0] != nullptr ? Done. Actually one of the try-bots ran into this bug. Thanks for the pointers.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mstarzinger@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rmcilroy@chromium.org Link to the patchset: https://codereview.chromium.org/2443923002/#ps20001 (title: "Addressed comment.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [interpreter] Ensure --debug-code works with snapshots. This makes sure that bytecode handlers are regenerated when debugging code within handlers is being requested. We cannot use the handlers baked into the snapshot in this case. R=rmcilroy@chromium.org ========== to ========== [interpreter] Ensure --debug-code works with snapshots. This makes sure that bytecode handlers are regenerated when debugging code within handlers is being requested. We cannot use the handlers baked into the snapshot in this case. R=rmcilroy@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [interpreter] Ensure --debug-code works with snapshots. This makes sure that bytecode handlers are regenerated when debugging code within handlers is being requested. We cannot use the handlers baked into the snapshot in this case. R=rmcilroy@chromium.org ========== to ========== [interpreter] Ensure --debug-code works with snapshots. This makes sure that bytecode handlers are regenerated when debugging code within handlers is being requested. We cannot use the handlers baked into the snapshot in this case. R=rmcilroy@chromium.org Committed: https://crrev.com/438c5eb28bb9a20782c03cd70dcdf3fa04f09e28 Cr-Commit-Position: refs/heads/master@{#40555} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/438c5eb28bb9a20782c03cd70dcdf3fa04f09e28 Cr-Commit-Position: refs/heads/master@{#40555} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
