Binds two IDs together.

Binds two IDs together.

This macro provides a mechanism to tell trace consumers
that two different IDs are actually identical and they
refer to the same thing.

For more context:
https://docs.google.com/a/google.com/document/d/1s0DKjNJk85hDuRp5IqujwZvPML-MPsyAtDeksMwBw7s/edit?usp=sharing

BUG=catapult:#2465

Committed: https://crrev.com/824aa1f16c39e840dffd25b78ee334ba0e58cbc1
Cr-Commit-Position: refs/heads/master@{#406648}

[chiniforooshan, 2016-07-12 16:11:59]

Description was changed from

==========
Binds two IDs together.

These macros provide a mechanism to tell trace consumers that two
different IDs, potentially from different processes, are actually
identical and they refer to the same thing.

For more context:
https://docs.google.com/a/google.com/document/d/1s0DKjNJk85hDuRp5IqujwZvPML-M...

BUG=catapult:#2465
==========

to

==========
Binds two IDs together.

These macros provide a mechanism to tell trace consumers that two
different IDs, potentially from different processes, are actually
identical and they refer to the same thing.

For more context:
https://docs.google.com/a/google.com/document/d/1s0DKjNJk85hDuRp5IqujwZvPML-M...

BUG=catapult:#2465
==========

[chiniforooshan, 2016-07-12 16:11:59]

chiniforooshan@chromium.org changed reviewers:
+ caseq@chromium.org, primiano@chromium.org

[chiniforooshan, 2016-07-12 16:16:19]

The CQ bit was checked by chiniforooshan@chromium.org to run a CQ dry run

[chiniforooshan, 2016-07-12 16:16:21]

The CQ bit was unchecked by chiniforooshan@chromium.org

[chiniforooshan, 2016-07-12 16:17:03]

The CQ bit was checked by chiniforooshan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 16:18:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 18:11:16]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 18:11:17]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[caseq, 2016-07-14 17:11:44]

let's add a test and a comment documenting usage, but otherwise lgtm.

I'm not entirely happy with the fact that ids and their scopes have to be
handled differently, but I guess there's not much we can do with present
TraceEvent structure. Let's fix this when transitioning to Tracing v2.

[chiniforooshan, 2016-07-18 19:55:42]

On 2016/07/14 17:11:44, caseq wrote:
> let's add a test and a comment documenting usage, but otherwise lgtm.
> 
> I'm not entirely happy with the fact that ids and their scopes have to be
> handled differently, but I guess there's not much we can do with present
> TraceEvent structure. Let's fix this when transitioning to Tracing v2.

Added a test and inline documentation.

Primiano: Could you please take a look? Thanks!

[Primiano Tucci (use gerrit), 2016-07-19 12:36:17]

Feels like a dejavu, what happened with all the past discussion and the past
approach (I don't even remember anymore what that was about)?

I think I missed some context / discussion in the process. What is the best way
for me to catch up with the last state of things here?

[chiniforooshan, 2016-07-19 19:37:42]

Description was changed from

==========
Binds two IDs together.

These macros provide a mechanism to tell trace consumers that two
different IDs, potentially from different processes, are actually
identical and they refer to the same thing.

For more context:
https://docs.google.com/a/google.com/document/d/1s0DKjNJk85hDuRp5IqujwZvPML-M...

BUG=catapult:#2465
==========

to

==========
Binds two IDs together.

This macro provides a mechanism to tell trace consumers
that two different IDs are actually identical and they
refer to the same thing.

For more context:
https://docs.google.com/a/google.com/document/d/1s0DKjNJk85hDuRp5IqujwZvPML-M...

BUG=catapult:#2465
==========

[chiniforooshan, 2016-07-19 19:39:40]

On 2016/07/19 12:36:17, Primiano Tucci wrote:
> Feels like a dejavu, what happened with all the past discussion and the past
> approach (I don't even remember anymore what that was about)?
> 
> I think I missed some context / discussion in the process. What is the best
way
> for me to catch up with the last state of things here?

As per our offline discussion, I took out the cross-process case to a separate
patch, since it may need more discussion. PTAL.

[Primiano Tucci (use gerrit), 2016-07-20 16:39:04]

LGTM with just some small comments about names.
I added them to the previous PS but they should apply anyways :)

Also make sure to update the trace event doc in:
https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAy...

https://codereview.chromium.org/2142023003/diff/40001/base/trace_event/trace_...
File base/trace_event/trace_event.h (right):

https://codereview.chromium.org/2142023003/diff/40001/base/trace_event/trace_...
base/trace_event/trace_event.h:359: trace_event_internal::TraceID
trace_event_trace_id(id, &dummy_flags); \
I think you don't need dummy_flags here and just use TraceID::DontMangle(id)

https://codereview.chromium.org/2142023003/diff/40001/base/trace_event/trace_...
base/trace_event/trace_event.h:360: trace_event_internal::TraceID
trace_event_bind_id(                    \
maybe calling these vars "source_id" and "target_id" instad of "trace_id" and
"bind_id" makes it easier to reason about?

https://codereview.chromium.org/2142023003/diff/40001/base/trace_event/trace_...
base/trace_event/trace_event.h:378: "bind_scope", trace_event_bind_id.scope(),
##__VA_ARGS__);      \
same here, I'd call this arg "target_scope" or "target_bind_scope" or
"bind_target_scope"

[chiniforooshan, 2016-07-20 18:41:37]

https://codereview.chromium.org/2142023003/diff/40001/base/trace_event/trace_...
File base/trace_event/trace_event.h (right):

https://codereview.chromium.org/2142023003/diff/40001/base/trace_event/trace_...
base/trace_event/trace_event.h:359: trace_event_internal::TraceID
trace_event_trace_id(id, &dummy_flags); \
On 2016/07/20 16:39:04, Primiano Tucci wrote:
> I think you don't need dummy_flags here and just use TraceID::DontMangle(id)

Done.

https://codereview.chromium.org/2142023003/diff/40001/base/trace_event/trace_...
base/trace_event/trace_event.h:360: trace_event_internal::TraceID
trace_event_bind_id(                    \
On 2016/07/20 16:39:04, Primiano Tucci wrote:
> maybe calling these vars "source_id" and "target_id" instad of "trace_id" and
> "bind_id" makes it easier to reason about?

Done.

https://codereview.chromium.org/2142023003/diff/40001/base/trace_event/trace_...
base/trace_event/trace_event.h:378: "bind_scope", trace_event_bind_id.scope(),
##__VA_ARGS__);      \
On 2016/07/20 16:39:04, Primiano Tucci wrote:
> same here, I'd call this arg "target_scope" or "target_bind_scope" or
> "bind_target_scope"

I changed the local variable names to {source, target}_id. But, I think it's
better to keep field name "bind_scope" in the args of the actual JSON event,
because we are stuck with the "bind_id" field name at least until tracing V2.

[chiniforooshan, 2016-07-20 18:42:02]

The CQ bit was checked by chiniforooshan@chromium.org

[chiniforooshan, 2016-07-20 18:42:03]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org,
primiano@chromium.org
Link to the patchset: https://codereview.chromium.org/2142023003/#ps100001
(title: "comments")

[commit-bot: I haz the power, 2016-07-20 18:42:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 20:12:33]

Description was changed from

==========
Binds two IDs together.

This macro provides a mechanism to tell trace consumers
that two different IDs are actually identical and they
refer to the same thing.

For more context:
https://docs.google.com/a/google.com/document/d/1s0DKjNJk85hDuRp5IqujwZvPML-M...

BUG=catapult:#2465
==========

to

==========
Binds two IDs together.

This macro provides a mechanism to tell trace consumers
that two different IDs are actually identical and they
refer to the same thing.

For more context:
https://docs.google.com/a/google.com/document/d/1s0DKjNJk85hDuRp5IqujwZvPML-M...

BUG=catapult:#2465
==========

[commit-bot: I haz the power, 2016-07-20 20:12:34]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-07-20 20:14:34]

Description was changed from

==========
Binds two IDs together.

This macro provides a mechanism to tell trace consumers
that two different IDs are actually identical and they
refer to the same thing.

For more context:
https://docs.google.com/a/google.com/document/d/1s0DKjNJk85hDuRp5IqujwZvPML-M...

BUG=catapult:#2465
==========

to

==========
Binds two IDs together.

This macro provides a mechanism to tell trace consumers
that two different IDs are actually identical and they
refer to the same thing.

For more context:
https://docs.google.com/a/google.com/document/d/1s0DKjNJk85hDuRp5IqujwZvPML-M...

BUG=catapult:#2465

Committed: https://crrev.com/824aa1f16c39e840dffd25b78ee334ba0e58cbc1
Cr-Commit-Position: refs/heads/master@{#406648}
==========

[commit-bot: I haz the power, 2016-07-20 20:14:35]

Patchset 6 (id:??) landed as
https://crrev.com/824aa1f16c39e840dffd25b78ee334ba0e58cbc1
Cr-Commit-Position: refs/heads/master@{#406648}