The TRACE_LINK_IDS macro

base/trace_event/trace_event.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_TRACE_EVENT_TRACE_EVENT_H_
 #define BASE_TRACE_EVENT_TRACE_EVENT_H_
 
 // This header file defines implementation details of how the trace macros in
 // trace_event_common.h collect and store trace events. Anything not
 // implementation-specific should go in trace_event_common.h instead of here.
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <string>
 
 #include "base/atomicops.h"
+#include "base/format_macros.h"

[Primiano Tucci (use gerrit), 2016/10/01 01:50:00]

remember to remove the unnecessary .h once you move the method out-of-line

[chiniforooshan, 2016/10/03 18:12:19]

Done.

 #include "base/macros.h"
+#include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "base/trace_event/common/trace_event_common.h"
 #include "base/trace_event/heap_profiler.h"
+#include "base/trace_event/trace_event_argument.h"

[Primiano Tucci (use gerrit), 2016/10/01 01:50:00]

Yeah please don't pull extra includes in this file or will very likely create a
mess in blink.

[chiniforooshan, 2016/10/03 18:12:19]

Done.

 #include "base/trace_event/trace_event_system_stats_monitor.h"
 #include "base/trace_event/trace_log.h"
 #include "build/build_config.h"
 
 // By default, const char* argument values are assumed to have long-lived scope
 // and will not be copied. Use this macro to force a const char* to be copied.
 #define TRACE_STR_COPY(str) \
     trace_event_internal::TraceStringWithCopy(str)
 
 // DEPRECATED: do not use: Consider using TRACE_ID_{GLOBAL, LOCAL} macros,
@@ skipping 337 matching lines @@
                                        trace_event_trace_id.id_flags();     \
       trace_event_internal::AddTraceEventWithThreadIdAndTimestamp(          \
           phase, INTERNAL_TRACE_EVENT_UID(category_group_enabled), name,    \
           trace_event_trace_id.scope(), trace_event_trace_id.raw_id(),      \
           thread_id, timestamp,                                             \
           trace_event_flags | TRACE_EVENT_FLAG_EXPLICIT_TIMESTAMP,          \
           trace_event_internal::kNoId, ##__VA_ARGS__);                      \
     }                                                                       \
   } while (0)
 
-// This macro ignores whether the bind_id is local, global, or mangled.
-#define INTERNAL_TRACE_EVENT_ADD_BIND_IDS(category_group, name, id, bind_id,  \
-                                          ...)                                \
+// The linked ID will not be mangled.
+#define INTERNAL_TRACE_EVENT_ADD_LINK_IDS(category_group, name, id1, id2)     \
     do {                                                                      \
       INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group);                 \
       if (INTERNAL_TRACE_EVENT_CATEGORY_GROUP_ENABLED_FOR_RECORDING_MODE()) { \
-        trace_event_internal::TraceID source_id((id));                        \
+        trace_event_internal::TraceID source_id((id1));                       \
         unsigned int source_flags = source_id.id_flags();                     \
-        trace_event_internal::TraceID target_id((bind_id));                   \
-        if (target_id.scope() == trace_event_internal::kGlobalScope) {        \
-          trace_event_internal::AddTraceEvent(                                \
-              TRACE_EVENT_PHASE_BIND_IDS,                                     \
-              INTERNAL_TRACE_EVENT_UID(category_group_enabled),               \
-              name, source_id.scope(), source_id.raw_id(),                    \
-              source_flags, target_id.raw_id(), ##__VA_ARGS__);               \
-        } else {                                                              \
-          trace_event_internal::AddTraceEvent(                                \
-              TRACE_EVENT_PHASE_BIND_IDS,                                     \
-              INTERNAL_TRACE_EVENT_UID(category_group_enabled),               \
-              name, source_id.scope(), source_id.raw_id(),                    \
-              source_flags, target_id.raw_id(),                               \
-              "bind_scope", target_id.scope(), ##__VA_ARGS__);                \
-        }                                                                     \
+        trace_event_internal::TraceID target_id((id2));                       \
+        trace_event_internal::AddTraceEvent(                                  \
+            TRACE_EVENT_PHASE_LINK_IDS,                                       \
+            INTERNAL_TRACE_EVENT_UID(category_group_enabled),                 \
+            name, source_id.scope(), source_id.raw_id(), source_flags,        \
+            trace_event_internal::kNoId,                                      \
+            "linked_id", target_id.dict_value());                             \

[Primiano Tucci (use gerrit), 2016/10/01 01:50:00]

don't you need a std::move here?

[chiniforooshan, 2016/10/03 18:12:19]

It's a return value. If I actually do std::move here I get a warning: "moving a
temporary object prevents copy elision".

This is similar to many other cases already in the code. e.g. we don't std::move
here:

https://cs.chromium.org/chromium/src/cc/debug/benchmark_instrumentation.cc?l=18.

       }                                                                       \
     } while (0)
 
 // Implementation detail: internal macro to create static category and add
 // metadata event if the category is enabled.
 #define INTERNAL_TRACE_EVENT_METADATA_ADD(category_group, name, ...)        \
   do {                                                                      \
     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group);                 \
     if (INTERNAL_TRACE_EVENT_CATEGORY_GROUP_ENABLED_FOR_RECORDING_MODE()) { \
       TRACE_EVENT_API_ADD_METADATA_EVENT(                                   \
@@ skipping 163 matching lines @@
   TraceID(GlobalId raw_id) : raw_id_(raw_id.raw_id()) {
     id_flags_ = TRACE_EVENT_FLAG_HAS_GLOBAL_ID;
   }
   TraceID(WithScope scoped_id) : scope_(scoped_id.scope()),
       raw_id_(scoped_id.raw_id()), id_flags_(scoped_id.id_flags()) {}
 
   unsigned long long raw_id() const { return raw_id_; }
   const char* scope() const { return scope_; }
   unsigned int id_flags() const { return id_flags_; }
 
+  std::unique_ptr<base::trace_event::TracedValue> dict_value() const {

[caseq, 2016/10/01 01:33:53]

Let's move it out-of-line, it's too big.

[Primiano Tucci (use gerrit), 2016/10/01 01:50:00]

+1 this is definitely beyon the limits for an inline function (see
https://google.github.io/styleguide/cppguide.html#Inline_Functions)

Also call it AsTracedValue() for uniformity with the other code.

also maybe the signature here schould be unique_ptr<ConvertableToTraceFormat>

TracedValue is not exposed by this header. I wonder if doing that would cause
problems in v8 or blink

[chiniforooshan, 2016/10/03 18:12:19]

Done.

+    std::unique_ptr<base::trace_event::TracedValue> value(
+        new base::trace_event::TracedValue());

[Primiano Tucci (use gerrit), 2016/10/01 01:50:00]

makeunique

[chiniforooshan, 2016/10/03 18:12:19]

Done.

+
+    if (scope_ != kGlobalScope)
+      value->SetString("scope", scope_);
+    switch (id_flags_) {
+      case TRACE_EVENT_FLAG_HAS_ID:
+        value->SetString(
+            "id",
+            base::StringPrintf("0x%" PRIx64, static_cast<uint64_t>(raw_id_)));
+        break;
+      case TRACE_EVENT_FLAG_HAS_GLOBAL_ID:
+        value->BeginDictionary("id2");
+        value->SetString(
+            "global",
+            base::StringPrintf("0x%" PRIx64, static_cast<uint64_t>(raw_id_)));
+        value->EndDictionary();
+        break;
+      case TRACE_EVENT_FLAG_HAS_LOCAL_ID:
+        value->BeginDictionary("id2");
+        value->SetString(
+            "local",
+            base::StringPrintf("0x%" PRIx64, static_cast<uint64_t>(raw_id_)));
+        value->EndDictionary();
+        break;
+      default:
+        NOTREACHED() << "Unrecognized ID flag";
+    }
+
+    return value;
+  }
+
  private:
   const char* scope_ = nullptr;
   unsigned long long raw_id_;
   unsigned int id_flags_ = TRACE_EVENT_FLAG_HAS_ID;
 };
 
 // Simple union to store various types as unsigned long long.
 union TraceValueUnion {
   bool as_bool;
   unsigned long long as_uint;
@@ skipping 564 matching lines @@
   const char* name_;
   IDType id_;
 
   DISALLOW_COPY_AND_ASSIGN(TraceScopedTrackableObject);
 };
 
 }  // namespace trace_event
 }  // namespace base
 
 #endif  // BASE_TRACE_EVENT_TRACE_EVENT_H_