Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2505493002:
[inspector] make console.assert much faster (Closed)
Description[inspector] make console.assert much faster
New console.assert implementation is faster then custom user implementation.
BUG=chromium:663845
R=dgozman@chromium.org
Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9
Committed: https://crrev.com/019d446c1e2d00ac61f1ecd4eeffdbe80a8cdb40
Cr-Original-Commit-Position: refs/heads/master@{#41227}
Cr-Commit-Position: refs/heads/master@{#41232}
Patch Set 1 #
Total comments: 2
Patch Set 2 : ready for review #Patch Set 3 : fix debug-script.js test #Patch Set 4 : do not run microtasks after internal function call #
Messages
Total messages: 44 (27 generated)
Dmitry, please take a look. I'll add Yang after your review :)
Let's extract part of this into separate patch. https://codereview.chromium.org/2505493002/diff/1/test/inspector/console/cons... File test/inspector/console/console-assert-expected.txt (right): https://codereview.chromium.org/2505493002/diff/1/test/inspector/console/cons... test/inspector/console/console-assert-expected.txt:19: url : Let's have an url. https://codereview.chromium.org/2505493002/diff/1/test/inspector/console/cons... File test/inspector/console/console-assert.js (right): https://codereview.chromium.org/2505493002/diff/1/test/inspector/console/cons... test/inspector/console/console-assert.js:5: print("Checks for console.assert"); Checks that console.assert produces api call with correct stack trace.
Description was changed from ========== [inspector] make console.assert 10 times faster then now - console.assert is slow because it requires call to native, we can optimize it for cases when value is true. - to achieve this and don't change location of assert call in sources we need to filter out v8-debugger internal scripts from reported stacks. BUG=chromium:663845 R=dgozman@chromium.org ========== to ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation and console.assert(false) is even faster then if(false) console.error(...). BUG=chromium:663845 R=dgozman@chromium.org ==========
Dmitry, please take a look! Let's make web great again :))
The CQ bit was checked by kozyatinskiy@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...
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/16656) 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...) v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/18282) v8_win_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng_triggered/bui...)
kozyatinskiy@chromium.org changed reviewers: + yangguo@chromium.org
Yang, please take a look!
!lgtm
The CQ bit was checked by kozyatinskiy@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation and console.assert(false) is even faster then if(false) console.error(...). BUG=chromium:663845 R=dgozman@chromium.org ========== to ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org ==========
On 2016/11/22 17:05:31, dgozman wrote: > !lgtm lgtm. This is not really elegant, but I don't know a better way that does not directly use V8 internals.
The CQ bit was checked by kozyatinskiy@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479910680057480,
"parent_rev": "f69a076fd7ed9ed543fc70e6d37110811182a87b", "commit_rev":
"99485378d28e64a48737e148c243af7e64a610a9"}
Message was sent while issue was closed.
Description was changed from ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org ========== to ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org ========== to ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227}
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2521323005/ by machenbach@chromium.org. The reason for reverting is: Speculative revert as there seems to be a layout test crash: https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/bui... Please reland if it doesn't get green..
Message was sent while issue was closed.
Description was changed from ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227} ========== to ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227} ==========
The CQ bit was checked by kozyatinskiy@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org, yangguo@chromium.org Link to the patchset: https://codereview.chromium.org/2505493002/#ps60001 (title: "do not run microtask after internal function call")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_precise_blink_rel on master.tryserver.blink (JOB_FAILED, no build URL)
Description was changed from ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227} ========== to ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org CQ_INCLUDE_TRYBOTS=v8_linux_chromium_gn_rel Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227} ==========
The CQ bit was checked by kozyatinskiy@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Your CL can not be processed by CQ because of: * Failed to parse additional trybots
Description was changed from ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org CQ_INCLUDE_TRYBOTS=v8_linux_chromium_gn_rel Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227} ========== to ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227} ==========
The CQ bit was checked by kozyatinskiy@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1479919964058040,
"parent_rev": "0014fa819cbd282b5f2a2b22b0ebbe4c9a0157d8", "commit_rev":
"6492d558472dd5128671a086d6b33ee8c618c2f5"}
Message was sent while issue was closed.
Description was changed from ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227} ========== to ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227} ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Cr-Commit-Position: refs/heads/master@{#41227} ========== to ========== [inspector] make console.assert much faster New console.assert implementation is faster then custom user implementation. BUG=chromium:663845 R=dgozman@chromium.org Committed: https://crrev.com/f658e41d864267fb9e99ea76faa7758b0b63d5c9 Committed: https://crrev.com/019d446c1e2d00ac61f1ecd4eeffdbe80a8cdb40 Cr-Original-Commit-Position: refs/heads/master@{#41227} Cr-Commit-Position: refs/heads/master@{#41232} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/019d446c1e2d00ac61f1ecd4eeffdbe80a8cdb40 Cr-Commit-Position: refs/heads/master@{#41232} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
