Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(346)

Issue 1035783002: Re-land issue 750183008 (Parallelize trace messages serialization) (Closed)

Created:
5 years, 9 months ago by caseq
Modified:
5 years, 9 months ago
Reviewers:
loislo
CC:
chromium-reviews, darin-cc_chromium.org, erikwright+watch_chromium.org, jam, tracing+reviews_chromium.org, wfh+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Re-land issue 750183008 (Parallelize trace messages serialization) This re-lands https://codereview.chromium.org/750183008 (originally by loislo@) with the output chunk size reduced down from 10M to 100K (close to what it used to be originally). The reason for telemetry failures of original change on Windows was the bug in websocket-client (https://github.com/liris/websocket-client/issues/163) that caused it to fragment Python heap when receiving a large frame. Original issue description: Move serialization into a worker thread. As a result IO thread will be able to send messages to the browser. The original implementation did serialization on IO thread and was not able to send the messages because ipc had is_blocked_on_write_ = true and had no chance to check the actual state of the channel. So the messages were collected in output_queue. Also the messages could be quite big and could block the IO thread for a long time. BUG=463572 TBR=dsinclair (as he already l-g-t-m-d the original patch) Committed: https://crrev.com/61b95998e904561aa31aa095982529543f2cbac5 Cr-Commit-Position: refs/heads/master@{#322391}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -14 lines) Patch
M base/trace_event/trace_event_impl.h View 3 chunks +7 lines, -3 lines 0 comments Download
M base/trace_event/trace_event_impl.cc View 7 chunks +22 lines, -9 lines 0 comments Download
M content/browser/tracing/tracing_controller_impl.cc View 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
caseq
5 years, 9 months ago (2015-03-25 17:28:29 UTC) #2
loislo
lgtm
5 years, 9 months ago (2015-03-26 14:57:48 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1035783002/1
5 years, 9 months ago (2015-03-26 14:59:45 UTC) #6
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 9 months ago (2015-03-26 15:56:51 UTC) #7
commit-bot: I haz the power
5 years, 9 months ago (2015-03-26 15:57:25 UTC) #8
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/61b95998e904561aa31aa095982529543f2cbac5
Cr-Commit-Position: refs/heads/master@{#322391}

Powered by Google App Engine
This is Rietveld 408576698