Roll WebRTC 13098:13104

Roll WebRTC 13098:13104

Changes: https://chromium.googlesource.com/external/webrtc/trunk/webrtc.git/+log/456c3c7..c0bec8f

As part of this roll, also adding more detailed tracing to JingleThreadWrapper.

After this CL, WebRTC starts posting messages with a location, which
includes a file name, function and line number:
https://codereview.webrtc.org/2019423006/

CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:linux_chromium_archive_rel_ng;tryserver.chromium.mac:mac_chromium_archive_rel_ng

Committed: https://crrev.com/18b0cb45a53fa7535547bd7002847ec8e6a8918e
Cr-Commit-Position: refs/heads/master@{#399338}

[Taylor_Brandstetter, 2016-06-02 22:26:55]

deadbeef@chromium.org changed reviewers:
+ primiano@chromium.org, sergeyu@chromium.org, tommi@chromium.org

[Taylor_Brandstetter, 2016-06-02 22:26:55]

primiano@: Please review trace_event changes. I'm adding the support for a trace
with 3 arguments (in this case, file, function and line number). If there's a
reason to limit it to 2, I can settle for just file and line number, and leave
trace_event unchanged.

tommi@, sergeyu@: Please review jingle changes. The CL linked in the description
changes the API of Thread and MessageQueue, requiring changes in
JingleThreadWrapper. I plan to land the linked CL, and then land this CL along
with a WebRTC roll immediately afterwards.

[Sergey Ulanov, 2016-06-03 09:12:21]

jingle/libjingle LGTM

[Primiano Tucci (use gerrit), 2016-06-03 09:21:26]

I am quite recluctant to add a 3rd argument to trace event. This will increase
the memory usage of tracing, can we deal with this differently without adding a
3rd arg?
Can you use the existing TRACE_TASK_EXECUTION?

[Taylor_Brandstetter, 2016-06-03 16:21:58]

On 2016/06/03 09:21:26, Primiano Tucci wrote:
> I am quite recluctant to add a 3rd argument to trace event. This will increase
> the memory usage of tracing, can we deal with this differently without adding
a
> 3rd arg?
> Can you use the existing TRACE_TASK_EXECUTION?

Ah, I see now that increasing kTraceMaxNumArgs will increase the size of every
trace event.

As I said, I can settle for two arguments (file and line number). I suppose I
could also concatenate two arguments; for example, have "src_file_and_line" and
"src_func". But ultimately it would be nice if TraceEvent could support an
arbitrary number of arguments.

[Primiano Tucci (use gerrit), 2016-06-03 17:10:20]

On 2016/06/03 16:21:58, Taylor_Brandstetter wrote:
> As I said, I can settle for two arguments (file and line number). I suppose I
> could also concatenate two arguments; for example, have "src_file_and_line"
and
> "src_func". 
Just remind that it will require you to do a strcat operation, make sure you
don't do that in a fastpath.
Also, remember that if you pass a non-long-lived string as argument, you have to
wrap it in TRACE_STR_COPY().

> But ultimately it would be nice if TraceEvent could support an
> arbitrary number of arguments.
We are in the process of restructuring tracing completely (and gradually). See
bit.ly/TracingV2.
In the long term we'll have a more stable API based on protobuf, instead of
arbitrary arguments being passed.

Also remember that today you have another alternative, TraceValue.
You can pass only one argument as follows:
scoped_ptr<TracedValue> value(new TracedValue);
value->SetString("function", foo);
value->SetString("file", bar);
value->SetInteger("line", bar);
TRACE_EVENT1("cat", "name", "task_info", std::move(value))

however from a perf viewpoint today that will be worse than doing a strcat (will
be solved by protobuf in TracingV2).
But that is the way other folks inject more structured args in the trace.

Honestly, unless you desperately need the line number (do you?), I'd just go for
TRACE_TASK_EXECUTION
Function name and file name should be enought to refer to an unambiguous piece
of code. 

If you really want all three, strcat might be the most sraightforward solution
for now.

[Taylor_Brandstetter, 2016-06-03 18:02:02]

On 2016/06/03 17:10:20, Primiano Tucci wrote:
> On 2016/06/03 16:21:58, Taylor_Brandstetter wrote:
> > As I said, I can settle for two arguments (file and line number). I suppose
I
> > could also concatenate two arguments; for example, have "src_file_and_line"
> and
> > "src_func". 
> Just remind that it will require you to do a strcat operation, make sure you
> don't do that in a fastpath.
> Also, remember that if you pass a non-long-lived string as argument, you have
to
> wrap it in TRACE_STR_COPY().
> 
> > But ultimately it would be nice if TraceEvent could support an
> > arbitrary number of arguments.
> We are in the process of restructuring tracing completely (and gradually). See
> bit.ly/TracingV2.
> In the long term we'll have a more stable API based on protobuf, instead of
> arbitrary arguments being passed.
> 
> Also remember that today you have another alternative, TraceValue.
> You can pass only one argument as follows:
> scoped_ptr<TracedValue> value(new TracedValue);
> value->SetString("function", foo);
> value->SetString("file", bar);
> value->SetInteger("line", bar);
> TRACE_EVENT1("cat", "name", "task_info", std::move(value))
> 
> however from a perf viewpoint today that will be worse than doing a strcat
(will
> be solved by protobuf in TracingV2).
> But that is the way other folks inject more structured args in the trace.
> 
> Honestly, unless you desperately need the line number (do you?), I'd just go
for
> TRACE_TASK_EXECUTION
> Function name and file name should be enought to refer to an unambiguous piece
> of code. 
> 
> If you really want all three, strcat might be the most sraightforward solution
> for now.

Function name and file name aren't quite enough, because there are places in
WebRTC where the same function may queue multiple messages.

For now, I'll concatenate the file and line number at compile time so it won't
have any performance impact, and I'll separate them later once TracingV2 is
available. Does that sound ok?

[Primiano Tucci (use gerrit), 2016-06-03 18:03:42]

On 2016/06/03 18:02:02, Taylor_Brandstetter wrote:
> On 2016/06/03 17:10:20, Primiano Tucci wrote:
> > On 2016/06/03 16:21:58, Taylor_Brandstetter wrote:
> > > As I said, I can settle for two arguments (file and line number). I
suppose
> I
> > > could also concatenate two arguments; for example, have
"src_file_and_line"
> > and
> > > "src_func". 
> > Just remind that it will require you to do a strcat operation, make sure you
> > don't do that in a fastpath.
> > Also, remember that if you pass a non-long-lived string as argument, you
have
> to
> > wrap it in TRACE_STR_COPY().
> > 
> > > But ultimately it would be nice if TraceEvent could support an
> > > arbitrary number of arguments.
> > We are in the process of restructuring tracing completely (and gradually).
See
> > bit.ly/TracingV2.
> > In the long term we'll have a more stable API based on protobuf, instead of
> > arbitrary arguments being passed.
> > 
> > Also remember that today you have another alternative, TraceValue.
> > You can pass only one argument as follows:
> > scoped_ptr<TracedValue> value(new TracedValue);
> > value->SetString("function", foo);
> > value->SetString("file", bar);
> > value->SetInteger("line", bar);
> > TRACE_EVENT1("cat", "name", "task_info", std::move(value))
> > 
> > however from a perf viewpoint today that will be worse than doing a strcat
> (will
> > be solved by protobuf in TracingV2).
> > But that is the way other folks inject more structured args in the trace.
> > 
> > Honestly, unless you desperately need the line number (do you?), I'd just go
> for
> > TRACE_TASK_EXECUTION
> > Function name and file name should be enought to refer to an unambiguous
piece
> > of code. 
> > 
> > If you really want all three, strcat might be the most sraightforward
solution
> > for now.
> 
> Function name and file name aren't quite enough, because there are places in
> WebRTC where the same function may queue multiple messages.
> 
> For now, I'll concatenate the file and line number at compile time so it won't
> have any performance impact, and I'll separate them later once TracingV2 is
> available. Does that sound ok?
sg. thanks

[Taylor_Brandstetter, 2016-06-10 21:20:58]

Description was changed from

==========
Adding more detailed tracing to JingleThreadWrapper.

After this CL, WebRTC starts posting messages with a location, which
includes a file name, function and line number:
https://codereview.webrtc.org/2019423006/
==========

to

==========
Roll WebRTC 13098:13104

Changes:
https://chromium.googlesource.com/external/webrtc/trunk/webrtc.git/+log/456c3...

As part of this roll, also adding more detailed tracing to JingleThreadWrapper.

After this CL, WebRTC starts posting messages with a location, which
includes a file name, function and line number:
https://codereview.webrtc.org/2019423006/

CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:linux_chromium_archive_rel_ng;tryserver.chromium.mac:mac_chromium_archive_rel_ng
==========

[Taylor_Brandstetter, 2016-06-10 21:21:13]

The CQ bit was checked by deadbeef@chromium.org

[Taylor_Brandstetter, 2016-06-10 21:21:13]

The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org
Link to the patchset: https://codereview.chromium.org/2037873003/#ps40001
(title: "Roll WebRTC")

[commit-bot: I haz the power, 2016-06-10 21:22:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2037873003/40001

[commit-bot: I haz the power, 2016-06-10 23:04:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-10 23:04:02]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Taylor_Brandstetter, 2016-06-10 23:28:29]

The CQ bit was checked by deadbeef@chromium.org

[commit-bot: I haz the power, 2016-06-10 23:29:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2037873003/40001

[commit-bot: I haz the power, 2016-06-11 00:57:15]

Description was changed from

==========
Roll WebRTC 13098:13104

Changes:
https://chromium.googlesource.com/external/webrtc/trunk/webrtc.git/+log/456c3...

As part of this roll, also adding more detailed tracing to JingleThreadWrapper.

After this CL, WebRTC starts posting messages with a location, which
includes a file name, function and line number:
https://codereview.webrtc.org/2019423006/

CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:linux_chromium_archive_rel_ng;tryserver.chromium.mac:mac_chromium_archive_rel_ng
==========

to

==========
Roll WebRTC 13098:13104

Changes:
https://chromium.googlesource.com/external/webrtc/trunk/webrtc.git/+log/456c3...

As part of this roll, also adding more detailed tracing to JingleThreadWrapper.

After this CL, WebRTC starts posting messages with a location, which
includes a file name, function and line number:
https://codereview.webrtc.org/2019423006/

CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:linux_chromium_archive_rel_ng;tryserver.chromium.mac:mac_chromium_archive_rel_ng
==========

[commit-bot: I haz the power, 2016-06-11 00:57:16]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-11 00:57:30]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-11 00:58:28]

Description was changed from

==========
Roll WebRTC 13098:13104

Changes:
https://chromium.googlesource.com/external/webrtc/trunk/webrtc.git/+log/456c3...

As part of this roll, also adding more detailed tracing to JingleThreadWrapper.

After this CL, WebRTC starts posting messages with a location, which
includes a file name, function and line number:
https://codereview.webrtc.org/2019423006/

CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:linux_chromium_archive_rel_ng;tryserver.chromium.mac:mac_chromium_archive_rel_ng
==========

to

==========
Roll WebRTC 13098:13104

Changes:
https://chromium.googlesource.com/external/webrtc/trunk/webrtc.git/+log/456c3...

As part of this roll, also adding more detailed tracing to JingleThreadWrapper.

After this CL, WebRTC starts posting messages with a location, which
includes a file name, function and line number:
https://codereview.webrtc.org/2019423006/

CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:linux_chromium_archive_rel_ng;tryserver.chromium.mac:mac_chromium_archive_rel_ng

Committed: https://crrev.com/18b0cb45a53fa7535547bd7002847ec8e6a8918e
Cr-Commit-Position: refs/heads/master@{#399338}
==========

[commit-bot: I haz the power, 2016-06-11 00:58:30]

Patchset 3 (id:??) landed as
https://crrev.com/18b0cb45a53fa7535547bd7002847ec8e6a8918e
Cr-Commit-Position: refs/heads/master@{#399338}