|
|
Description[Tracing] V8 Tracing Controller - Fix async trace event bug
Usage of hex IO manipulator for async event IDs corrupts future decimal number
outputs.
BUG=v8:5261
Committed: https://crrev.com/94ad974df8434844ea179f0a57b52b94dc7bc760
Cr-Commit-Position: refs/heads/master@{#38331}
Patch Set 1 #Patch Set 2 : Attempt fix for windows pointer output #Patch Set 3 : Fix windows pointer output #
Total comments: 4
Patch Set 4 : Merge async test as part of TestJSONTraceWriter #
Messages
Total messages: 37 (25 generated)
The CQ bit was checked by rskang@google.com 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_win64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/11702) v8_win64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng_triggered/b...)
The CQ bit was checked by rskang@google.com 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_win64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/11706) v8_win64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng_triggered/b...)
The CQ bit was checked by rskang@google.com 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.
rskang@google.com changed reviewers: + fmeawad@google.com, lpy@chromium.org, mattloring@google.com
JSONTraceWriter output for async trace events had a bug: Usage of hex IO manipulator for async event IDs corrupts future decimal integer outputs. This bug wasn't caught by tests as async events weren't tested previously. This CL adds a test as well as the fix for it.
lgtm
lgtm
On 2016/08/03 02:48:27, mattloring wrote: > lgtm Please create a bug, and set BUG=v8:...
fmeawad@chromium.org changed reviewers: + fmeawad@chromium.org
LGTM with comments. https://codereview.chromium.org/2200113003/diff/40001/test/cctest/libplatform... File test/cctest/libplatform/test-tracing.cc (right): https://codereview.chromium.org/2200113003/diff/40001/test/cctest/libplatform... test/cctest/libplatform/test-tracing.cc:312: TEST(TestAsyncTraceEvent) { All test names are based on Classes and not trace event types, can you merge the test code to be part of the TraceWriter test? https://codereview.chromium.org/2200113003/diff/40001/test/cctest/libplatform... test/cctest/libplatform/test-tracing.cc:335: TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("v8", "Test2", p); Can you add an END for these trace events?
Also keep 80-column in CL description please.
The CQ bit was checked by rskang@google.com 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 ========== [Tracing] V8 Tracing Controller - Fix async trace event bug Usage of hex IO manipulator for async event IDs corrupts future decimal number outputs. BUG= ========== to ========== [Tracing] V8 Tracing Controller - Fix async trace event bug Usage of hex IO manipulator for async event IDs corrupts future decimal number outputs. BUG=v8:5261 ==========
Description was changed from ========== [Tracing] V8 Tracing Controller - Fix async trace event bug Usage of hex IO manipulator for async event IDs corrupts future decimal number outputs. BUG= ========== to ========== [Tracing] V8 Tracing Controller - Fix async trace event bug Usage of hex IO manipulator for async event IDs corrupts future decimal number outputs. BUG=v8:5261 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Fixed 80-line character limit for CL description. Added BUG id. https://codereview.chromium.org/2200113003/diff/40001/test/cctest/libplatform... File test/cctest/libplatform/test-tracing.cc (right): https://codereview.chromium.org/2200113003/diff/40001/test/cctest/libplatform... test/cctest/libplatform/test-tracing.cc:312: TEST(TestAsyncTraceEvent) { On 2016/08/03 06:48:02, fmeawad wrote: > All test names are based on Classes and not trace event types, can you merge the > test code to be part of the TraceWriter test? Done. https://codereview.chromium.org/2200113003/diff/40001/test/cctest/libplatform... test/cctest/libplatform/test-tracing.cc:335: TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("v8", "Test2", p); On 2016/08/03 06:48:02, fmeawad wrote: > Can you add an END for these trace events? Not needed after merging into TestJSONTraceWriter.
mattloring@google.com changed reviewers: + yangguo@chromium.org
Added @Yang to reviewers.
On 2016/08/04 00:57:37, rskang wrote: > Added @Yang to reviewers. lgtm.
The CQ bit was checked by rskang@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from mattloring@google.com, fmeawad@chromium.org Link to the patchset: https://codereview.chromium.org/2200113003/#ps60001 (title: "Merge async test as part of TestJSONTraceWriter")
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 ========== [Tracing] V8 Tracing Controller - Fix async trace event bug Usage of hex IO manipulator for async event IDs corrupts future decimal number outputs. BUG=v8:5261 ========== to ========== [Tracing] V8 Tracing Controller - Fix async trace event bug Usage of hex IO manipulator for async event IDs corrupts future decimal number outputs. BUG=v8:5261 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== [Tracing] V8 Tracing Controller - Fix async trace event bug Usage of hex IO manipulator for async event IDs corrupts future decimal number outputs. BUG=v8:5261 ========== to ========== [Tracing] V8 Tracing Controller - Fix async trace event bug Usage of hex IO manipulator for async event IDs corrupts future decimal number outputs. BUG=v8:5261 Committed: https://crrev.com/94ad974df8434844ea179f0a57b52b94dc7bc760 Cr-Commit-Position: refs/heads/master@{#38331} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/94ad974df8434844ea179f0a57b52b94dc7bc760 Cr-Commit-Position: refs/heads/master@{#38331} |