Allow tracing in third_party libraries

base/debug/trace_event.h

 // Copyright (c) 2011 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.
 
 // Trace events are for tracking application performance and resource usage.
 // Macros are provided to track:
 //    Begin and end of function calls
 //    Counters
 //
 // Events are issued against categories. Whereas LOG's
@@ skipping 142 matching lines @@
 #include <vector>
 
 #include "base/callback.h"
 #include "base/hash_tables.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/string_util.h"
 #include "base/synchronization/lock.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "base/timer.h"
 
+////////////////////////////////////////////////////////////////////////////////
+/// Customize these defines to call through to platform tracing APIs.

[nduca, 2012/01/11 21:49:11]

You know, I'm starting to think we should make trace_event.h be a stub code that
#defines the TRACE_EVENT_API macros
#includes trace_event_macros.h
#includes trace_event_impl.h

Sorry it took me this long to figure out what you meant.

This would allow you to put nice documentation on the trace_event_macros.h file
that explains what it does. Something like "This header is designed to be give
you trace_event macros without specifying how the events actually get collected
and stored. If you need to expose trace event to some other universe, you can
copy-and-paste this file and just implement API* macros."

I'm not sure the TRACE_EVENT_NAMESPACE adds a lot of value here. That seems
specific to the binding of the macros to the base implementation, no? That is,
webkit wont even need that, right? There, we'll just point the macros at
something completely whacko.

[jbates, 2012/01/11 22:44:35]

I think this sounds good, but this is a step-2 feedback, right? I'm deferring
anything related to splitting up files to a follow-on CL so that we can clearly
see the refactoring changes first.
There are some local types that get copied to other libraries such as
TraceStringWithCopy, which undoubtedly would be put inside a namespace in
third_party libraries. To reduce the hand-editing required to fix up the _export
header, I tried to put everything that will need hand-editing in this section.
Alternatively I could just choose a namespace like "trace_event_internal::" and
throw all this stuff in there -- come to think of it that is a lot cleaner.
Sound good?

[jbates, 2012/01/11 23:52:44]

Removed TRACE_EVENT_NAMESPACE.

+
+#define TRACE_EVENT_NAMESPACE base::debug

[jar (doing other things), 2012/01/12 00:15:36]

What does this macro help you with?

[jbates, 2012/01/12 00:52:20]

Done. (removed)

+
+// volatile bool*
+//     TRACE_EVENT_API_GET_CATEGORY_ENABLED(const char* category_name)
+#define TRACE_EVENT_API_GET_CATEGORY_ENABLED \
+    TRACE_EVENT_NAMESPACE::TraceLog::GetCategoryEnabled
+
+// Returns the threshold_begin_id used by TRACE_IF_LONGER_THAN macros.
+// int TRACE_EVENT_API_ADD_TRACE_EVENT(
+//                    char phase,
+//                    const volatile bool* category_enabled,
+//                    const char* name,
+//                    uint64 id,
+//                    int num_args,
+//                    const char** arg_names,
+//                    const uint8* arg_types,
+//                    const uint64* arg_values,
+//                    int threshold_begin_id,
+//                    int64 threshold,
+//                    uint8 flags)
+#define TRACE_EVENT_API_ADD_TRACE_EVENT \
+    TRACE_EVENT_NAMESPACE::TraceLog::GetInstance()->AddTraceEvent
+
+// void TRACE_EVENT_API_ADD_COUNTER_EVENT(
+//                    const volatile bool* category_enabled,
+//                    const char* name,
+//                    uint64 id,
+//                    const char* arg1_name, int32 arg1_val,
+//                    const char* arg2_name, int32 arg2_val,
+//                    uint8 flags)
+#define TRACE_EVENT_API_ADD_COUNTER_EVENT \
+    TRACE_EVENT_NAMESPACE::TraceLog::GetInstance()->AddCounterEvent
+
+// Mangle |pointer| with a process ID hash so that if |pointer| occurs on more
+// than one process, it will not collide in the trace data.
+// uint64 TRACE_EVENT_API_GET_ID_FROM_POINTER(void* pointer)
+#define TRACE_EVENT_API_GET_ID_FROM_POINTER \
+    TRACE_EVENT_NAMESPACE::TraceLog::GetInstance()->GetInterProcessID
+
+////////////////////////////////////////////////////////////////////////////////
+
 // 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) base::debug::TraceValue::StringWithCopy(str)
+#define TRACE_STR_COPY(str) \
+    TRACE_EVENT_NAMESPACE::internal::TraceStringWithCopy(str)
 
 // Older style trace macros with explicit id and extra data
 // Only these macros result in publishing data to ETW as currently implemented.
 #define TRACE_EVENT_BEGIN_ETW(name, id, extra) \
     base::debug::TraceLog::AddTraceEventEtw( \
         TRACE_EVENT_PHASE_BEGIN, \
         name, reinterpret_cast<const void*>(id), extra)
 
 #define TRACE_EVENT_END_ETW(name, id, extra) \
     base::debug::TraceLog::AddTraceEventEtw( \
         TRACE_EVENT_PHASE_END, \
         name, reinterpret_cast<const void*>(id), extra)
 
 #define TRACE_EVENT_INSTANT_ETW(name, id, extra) \
     base::debug::TraceLog::AddTraceEventEtw( \
         TRACE_EVENT_PHASE_INSTANT, \
         name, reinterpret_cast<const void*>(id), extra)
 
 // Records a pair of begin and end events called "name" for the current
 // scope, with 0, 1 or 2 associated arguments. If the category is not
 // enabled, then this does nothing.
 // - category and name strings must have application lifetime (statics or
 //   literals). They may not include " chars.
 #define TRACE_EVENT0(category, name) \
-    TRACE_EVENT1(category, name, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_SCOPED(category, name)
 #define TRACE_EVENT1(category, name, arg1_name, arg1_val) \
-    TRACE_EVENT2(category, name, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_SCOPED(category, name, arg1_name, arg1_val)
 #define TRACE_EVENT2(category, name, arg1_name, arg1_val, arg2_name, arg2_val) \
-    INTERNAL_TRACE_EVENT_ADD_SCOPED( \
-        category, name, arg1_name, arg1_val, arg2_name, arg2_val)
+    INTERNAL_TRACE_EVENT_ADD_SCOPED(category, name, arg1_name, arg1_val, \
+        arg2_name, arg2_val)
 
 // Same as TRACE_EVENT except that they are not included in official builds.
 #ifdef OFFICIAL_BUILD
 #define UNSHIPPED_TRACE_EVENT0(category, name) (void)0
 #define UNSHIPPED_TRACE_EVENT1(category, name, arg1_name, arg1_val) (void)0
 #define UNSHIPPED_TRACE_EVENT2(category, name, arg1_name, arg1_val, \
                                arg2_name, arg2_val) (void)0
 #define UNSHIPPED_TRACE_EVENT_INSTANT0(category, name) (void)0
 #define UNSHIPPED_TRACE_EVENT_INSTANT1(category, name, arg1_name, arg1_val) \
     (void)0
@@ skipping 16 matching lines @@
     TRACE_EVENT_INSTANT2(category, name, arg1_name, arg1_val, \
                          arg2_name, arg2_val)
 #endif
 
 // Records a single event called "name" immediately, with 0, 1 or 2
 // associated arguments. If the category is not enabled, then this
 // does nothing.
 // - category and name strings must have application lifetime (statics or
 //   literals). They may not include " chars.
 #define TRACE_EVENT_INSTANT0(category, name) \
-    TRACE_EVENT_INSTANT1(category, name, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_INSTANT, \
+        category, name, TRACE_EVENT_FLAG_NONE)
 #define TRACE_EVENT_INSTANT1(category, name, arg1_name, arg1_val) \
-    TRACE_EVENT_INSTANT2(category, name, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_INSTANT, \
+        category, name, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val)
 #define TRACE_EVENT_INSTANT2(category, name, arg1_name, arg1_val, \
         arg2_name, arg2_val) \
     INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_INSTANT, \
-        category, name, arg1_name, arg1_val, arg2_name, arg2_val, \
-        TRACE_EVENT_FLAG_NONE)
+        category, name, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val, \
+        arg2_name, arg2_val)
 #define TRACE_EVENT_COPY_INSTANT0(category, name) \
-    TRACE_EVENT_COPY_INSTANT1(category, name, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_INSTANT, \
+        category, name, TRACE_EVENT_FLAG_COPY)
 #define TRACE_EVENT_COPY_INSTANT1(category, name, arg1_name, arg1_val) \
-    TRACE_EVENT_COPY_INSTANT2(category, name, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_INSTANT, \
+        category, name, TRACE_EVENT_FLAG_COPY, arg1_name, arg1_val)
 #define TRACE_EVENT_COPY_INSTANT2(category, name, arg1_name, arg1_val, \
         arg2_name, arg2_val) \
     INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_INSTANT, \
-        category, name, \
-        arg1_name, base::debug::TraceValue::ForceCopy(arg1_val), \
-        arg2_name, base::debug::TraceValue::ForceCopy(arg2_val), \
-        TRACE_EVENT_FLAG_COPY)
+        category, name, TRACE_EVENT_FLAG_COPY, arg1_name, arg1_val, \
+        arg2_name, arg2_val)
 
 // Records a single BEGIN event called "name" immediately, with 0, 1 or 2
 // associated arguments. If the category is not enabled, then this
 // does nothing.
 // - category and name strings must have application lifetime (statics or
 //   literals). They may not include " chars.
 #define TRACE_EVENT_BEGIN0(category, name) \
-    TRACE_EVENT_BEGIN1(category, name, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, \
+        category, name, TRACE_EVENT_FLAG_NONE)
 #define TRACE_EVENT_BEGIN1(category, name, arg1_name, arg1_val) \
-    TRACE_EVENT_BEGIN2(category, name, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, \
+        category, name, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val)
 #define TRACE_EVENT_BEGIN2(category, name, arg1_name, arg1_val, \
         arg2_name, arg2_val) \
     INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, \
-        category, name, arg1_name, arg1_val, arg2_name, arg2_val, \
-        TRACE_EVENT_FLAG_NONE)
+        category, name, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val, \
+        arg2_name, arg2_val)
 #define TRACE_EVENT_COPY_BEGIN0(category, name) \
-    TRACE_EVENT_COPY_BEGIN1(category, name, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, \
+        category, name, TRACE_EVENT_FLAG_COPY)
 #define TRACE_EVENT_COPY_BEGIN1(category, name, arg1_name, arg1_val) \
-    TRACE_EVENT_COPY_BEGIN2(category, name, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, \
+        category, name, TRACE_EVENT_FLAG_COPY, arg1_name, arg1_val)
 #define TRACE_EVENT_COPY_BEGIN2(category, name, arg1_name, arg1_val, \
         arg2_name, arg2_val) \
     INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, \
-        category, name, \
-        arg1_name, base::debug::TraceValue::ForceCopy(arg1_val), \
-        arg2_name, base::debug::TraceValue::ForceCopy(arg2_val), \
-        TRACE_EVENT_FLAG_COPY)
+        category, name, TRACE_EVENT_FLAG_COPY, arg1_name, arg1_val, \
+        arg2_name, arg2_val)
 
 // Records a single END event for "name" immediately. If the category
 // is not enabled, then this does nothing.
 // - category and name strings must have application lifetime (statics or
 //   literals). They may not include " chars.
 #define TRACE_EVENT_END0(category, name) \
-    TRACE_EVENT_END1(category, name, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_END, \
+        category, name, TRACE_EVENT_FLAG_NONE)
 #define TRACE_EVENT_END1(category, name, arg1_name, arg1_val) \
-    TRACE_EVENT_END2(category, name, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_END, \
+        category, name, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val)
 #define TRACE_EVENT_END2(category, name, arg1_name, arg1_val, \
         arg2_name, arg2_val) \
     INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_END, \
-        category, name, arg1_name, arg1_val, arg2_name, arg2_val, \
-        TRACE_EVENT_FLAG_NONE)
+        category, name, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val, \
+        arg2_name, arg2_val)
 #define TRACE_EVENT_COPY_END0(category, name) \
-    TRACE_EVENT_COPY_END1(category, name, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_END, \
+        category, name, TRACE_EVENT_FLAG_COPY)
 #define TRACE_EVENT_COPY_END1(category, name, arg1_name, arg1_val) \
-    TRACE_EVENT_COPY_END2(category, name, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_END, \
+        category, name, TRACE_EVENT_FLAG_COPY, arg1_name, arg1_val)
 #define TRACE_EVENT_COPY_END2(category, name, arg1_name, arg1_val, \
         arg2_name, arg2_val) \
     INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_END, \
-        category, name, \
-        arg1_name, base::debug::TraceValue::ForceCopy(arg1_val), \
-        arg2_name, base::debug::TraceValue::ForceCopy(arg2_val), \
-        TRACE_EVENT_FLAG_COPY)
+        category, name, TRACE_EVENT_FLAG_COPY, arg1_name, arg1_val, \
+        arg2_name, arg2_val)
 
 // Time threshold event:
 // Only record the event if the duration is greater than the specified
 // threshold_us (time in microseconds).
 // Records a pair of begin and end events called "name" for the current
 // scope, with 0, 1 or 2 associated arguments. If the category is not
 // enabled, then this does nothing.
 // - category and name strings must have application lifetime (statics or
 //   literals). They may not include " chars.
 #define TRACE_EVENT_IF_LONGER_THAN0(threshold_us, category, name) \
-    TRACE_EVENT_IF_LONGER_THAN1(threshold_us, category, name, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_SCOPED_IF_LONGER_THAN(threshold_us, category, name)
 #define TRACE_EVENT_IF_LONGER_THAN1( \
-        threshold_us, category, name, arg1_name, arg1_val) \
-    TRACE_EVENT_IF_LONGER_THAN2(threshold_us, category, name, \
-                           arg1_name, arg1_val, NULL, 0)
+    threshold_us, category, name, arg1_name, arg1_val) \
+    INTERNAL_TRACE_EVENT_ADD_SCOPED_IF_LONGER_THAN( \
+        threshold_us, category, name, arg1_name, arg1_val)
 #define TRACE_EVENT_IF_LONGER_THAN2( \
     threshold_us, category, name, arg1_name, arg1_val, arg2_name, arg2_val) \
-    INTERNAL_TRACE_EVENT_ADD_SCOPED_IF_LONGER_THAN(threshold_us, \
-        category, name, arg1_name, arg1_val, arg2_name, arg2_val)
+    INTERNAL_TRACE_EVENT_ADD_SCOPED_IF_LONGER_THAN( \
+        threshold_us, category, name, arg1_name, arg1_val, arg2_name, arg2_val)
 
 // Records the value of a counter called "name" immediately. Value
 // must be representable as a 32 bit integer.
 // - category and name strings must have application lifetime (statics or
 //   literals). They may not include " chars.
 #define TRACE_COUNTER1(category, name, value) \
     TRACE_COUNTER2(category, name, "value", value, NULL, 0)
 #define TRACE_COPY_COUNTER1(category, name, value) \
     TRACE_COPY_COUNTER2(category, name, "value", value, NULL, 0)
 
@@ skipping 54 matching lines @@
 // Records a single START event called "name" immediately, with 0, 1 or 2
 // associated arguments. If the category is not enabled, then this
 // does nothing.
 // - category and name strings must have application lifetime (statics or
 //   literals). They may not include " chars.
 // - |id| is used to match the START event with the FINISH event. It must either
 //   be a pointer or an integer value up to 64 bits. If it's a pointer, the bits
 //   will be xored with a hash of the process ID so that the same pointer on
 //   two different processes will not collide.
 #define TRACE_EVENT_START0(category, name, id) \
-    TRACE_EVENT_START1(category, name, id, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_START, \
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID)
 #define TRACE_EVENT_START1(category, name, id, arg1_name, arg1_val) \
-    TRACE_EVENT_START2(category, name, id, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_START, \
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID, arg1_name, arg1_val)
 #define TRACE_EVENT_START2(category, name, id, arg1_name, arg1_val, \
         arg2_name, arg2_val) \
     INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_START, \
-        category, name, id, arg1_name, arg1_val, arg2_name, arg2_val, \
-        TRACE_EVENT_FLAG_HAS_ID)
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID, \
+        arg1_name, arg1_val, arg2_name, arg2_val)
 #define TRACE_EVENT_COPY_START0(category, name, id) \
-    TRACE_EVENT_COPY_START1(category, name, id, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_START, \
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID | TRACE_EVENT_FLAG_COPY)
 #define TRACE_EVENT_COPY_START1(category, name, id, arg1_name, arg1_val) \
-    TRACE_EVENT_COPY_START2(category, name, id, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_START, \
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID | TRACE_EVENT_FLAG_COPY, \
+        arg1_name, arg1_val)
 #define TRACE_EVENT_COPY_START2(category, name, id, arg1_name, arg1_val, \
         arg2_name, arg2_val) \
     INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_START, \
-        category, name, id, \
-        arg1_name, base::debug::TraceValue::ForceCopy(arg1_val), \
-        arg2_name, base::debug::TraceValue::ForceCopy(arg2_val), \
-        TRACE_EVENT_FLAG_HAS_ID | TRACE_EVENT_FLAG_COPY)
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID | TRACE_EVENT_FLAG_COPY, \
+        arg1_name, arg1_val, arg2_name, arg2_val)
 
 // Records a single FINISH event for "name" immediately. If the category
 // is not enabled, then this does nothing.
 #define TRACE_EVENT_FINISH0(category, name, id) \
-    TRACE_EVENT_FINISH1(category, name, id, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_FINISH, \
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID)
 #define TRACE_EVENT_FINISH1(category, name, id, arg1_name, arg1_val) \
-    TRACE_EVENT_FINISH2(category, name, id, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_FINISH, \
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID, arg1_name, arg1_val)
 #define TRACE_EVENT_FINISH2(category, name, id, arg1_name, arg1_val, \
         arg2_name, arg2_val) \
     INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_FINISH, \
-        category, name, id, arg1_name, arg1_val, arg2_name, arg2_val, \
-        TRACE_EVENT_FLAG_HAS_ID)
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID, \
+        arg1_name, arg1_val, arg2_name, arg2_val)
 #define TRACE_EVENT_COPY_FINISH0(category, name, id) \
-    TRACE_EVENT_COPY_FINISH1(category, name, id, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_FINISH, \
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID | TRACE_EVENT_FLAG_COPY)
 #define TRACE_EVENT_COPY_FINISH1(category, name, id, arg1_name, arg1_val) \
-    TRACE_EVENT_COPY_FINISH2(category, name, id, arg1_name, arg1_val, NULL, 0)
+    INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_FINISH, \
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID | TRACE_EVENT_FLAG_COPY, \
+        arg1_name, arg1_val)
 #define TRACE_EVENT_COPY_FINISH2(category, name, id, arg1_name, arg1_val, \
         arg2_name, arg2_val) \
     INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_FINISH, \
-        category, name, id, \
-        arg1_name, base::debug::TraceValue::ForceCopy(arg1_val), \
-        arg2_name, base::debug::TraceValue::ForceCopy(arg2_val), \
-        TRACE_EVENT_FLAG_HAS_ID | TRACE_EVENT_FLAG_COPY)
+        category, name, id, TRACE_EVENT_FLAG_HAS_ID | TRACE_EVENT_FLAG_COPY, \
+        arg1_name, arg1_val, arg2_name, arg2_val)
 
 
 // Implementation detail: trace event macros create temporary variables
 // to keep instrumentation overhead low. These macros give each temporary
 // variable a unique name based on the line number to prevent name collissions.
 #define INTERNAL_TRACE_EVENT_UID3(a,b) \
     trace_event_unique_##a##b
 #define INTERNAL_TRACE_EVENT_UID2(a,b) \
     INTERNAL_TRACE_EVENT_UID3(a,b)
 #define INTERNAL_TRACE_EVENT_UID(name_prefix) \
     INTERNAL_TRACE_EVENT_UID2(name_prefix, __LINE__)
 
 // Implementation detail: internal macro to create static category.
 // - ANNOTATE_BENIGN_RACE, see Thread Safety above.
 #define INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category) \
-    static const base::debug::TraceCategory* \
+    static const volatile bool* \

[jar (doing other things), 2012/01/12 00:15:36]

What does the use of volatile do for you?  (for most users, not doing hardware
register coding, this keyword is misused).

[jbates, 2012/01/12 00:52:20]

It's useful in this case, because it tells the compiler that the load of this
bool can't be moved from the inside to the outside of a loop.

[jar (doing other things), 2012/01/12 02:20:03]

I don't think volatile has such ordering semantics.  Can you give me a pointer
to where this is stated? 

On 2012/01/12 00:52:20, jbates wrote:

[joth, 2012/01/12 16:14:46]

Not a normative reference, but very informative:
http://kerneltrap.org/Linux/Volatile_Superstition


"""
Volatile doesn't mean it can't be reordered; volatile means the
accesses can't be eliminated.
...

So if "volatile" makes a difference, it is invariably a sign of a bug in 
serialization (the one exception is for IO - we use "volatile" to avoid 
having to use inline asm for IO on x86) - and for "random values" like 
jiffies).
"""

Not eliminating the memory access is key: I think these bools actually do fall
into this "random values" corner case, where reording is fairly harmless, but
you don't want any accesses to be eliminated.
i.e. absolutely nothing 'bad' would occur w.r.t the production code if this code
happens to read the 'wrong' value on every iteration. The goal here is to stop
the compiler eliminating the inner-loop memory accesses. (whether that's
actually a good trade off though I can't call. Linus's comments about volatile
resulting in crap code generation, which will be a cost in *all* the other
non-loop call-sites!, maybe more important than having the category 'enablement'
be observed by any code already in its loop?)

         INTERNAL_TRACE_EVENT_UID(catstatic) = NULL; \
     ANNOTATE_BENIGN_RACE(&INTERNAL_TRACE_EVENT_UID(catstatic), \
                          "trace_event category"); \
     if (!INTERNAL_TRACE_EVENT_UID(catstatic)) \
       INTERNAL_TRACE_EVENT_UID(catstatic) = \
-          base::debug::TraceLog::GetCategory(category);
+          TRACE_EVENT_API_GET_CATEGORY_ENABLED(category);
 
 // Implementation detail: internal macro to create static category and add
 // event if the category is enabled.
-#define INTERNAL_TRACE_EVENT_ADD( \
-      phase, category, name, arg1_name, arg1_val, arg2_name, arg2_val, flags) \
+#define INTERNAL_TRACE_EVENT_ADD(phase, category, name, flags, ...) \

[jar (doing other things), 2012/01/12 00:15:36]

Is ellipsis (vararg) defined for the preprocessor???  I thought this was only
just part of the emerging standard... and I'm a little surprised to see it used
(as it may make porting harder).  

Am I too old school?

[jbates, 2012/01/12 00:52:20]

It's used in base/logging.h so I'm hoping I can use it here to avoid a lot of
macro code duplication.

[jar (doing other things), 2012/01/12 02:20:03]

Interesting.  OK.  Thanks for the example.  It is still surprising to me.

On 2012/01/12 00:52:20, jbates wrote:

     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category); \
-    if (INTERNAL_TRACE_EVENT_UID(catstatic)->enabled) { \
-      base::debug::TraceLog::GetInstance()->AddTraceEvent( \
-          phase, INTERNAL_TRACE_EVENT_UID(catstatic), \
-          name, 0, arg1_name, arg1_val, arg2_name, arg2_val, -1, 0, flags); \
+    if (*INTERNAL_TRACE_EVENT_UID(catstatic)) { \
+      TRACE_EVENT_NAMESPACE::internal::AddTraceEvent( \
+          phase, INTERNAL_TRACE_EVENT_UID(catstatic), name, 0, flags, \
+          ##__VA_ARGS__); \

[jar (doing other things), 2012/01/12 00:15:36]

... and you're also using the ## gcc compatibility hack.

Again, for portability, I'm surprised we're doing this stuff.  Can you give me
any pointers?

[jbates, 2012/01/12 00:52:20]

I just stole this from base/logging.h, which did not appear to be a platform
dependent implementation (surprising to me as well).

     }
 
 // Implementation detail: internal macro to create static category and
 // add the counter event if it is enabled.
 #define INTERNAL_TRACE_EVENT_ADD_COUNTER( \
       category, name, id, arg1_name, arg1_val, arg2_name, arg2_val, flags) \
     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category); \
-    if (INTERNAL_TRACE_EVENT_UID(catstatic)->enabled) { \
-      base::debug::TraceLog::GetInstance()->AddCounterEvent( \
+    if (*INTERNAL_TRACE_EVENT_UID(catstatic)) { \
+      TRACE_EVENT_API_ADD_COUNTER_EVENT( \
           INTERNAL_TRACE_EVENT_UID(catstatic), \
-          name, id, arg1_name, arg1_val, arg2_name, arg2_val, flags); \
+          name, TRACE_EVENT_NAMESPACE::TraceID(id).data(), \
+          arg1_name, arg1_val, arg2_name, arg2_val, flags); \
     }
 
 // Implementation detail: internal macro to create static category and add begin
 // event if the category is enabled. Also adds the end event when the scope
 // ends.
-#define INTERNAL_TRACE_EVENT_ADD_SCOPED( \
-      category, name, arg1_name, arg1_val, arg2_name, arg2_val) \
+#define INTERNAL_TRACE_EVENT_ADD_SCOPED(category, name, ...) \
     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category); \
-    base::debug::internal::TraceEndOnScopeClose \
+    TRACE_EVENT_NAMESPACE::internal::TraceEndOnScopeClose \
         INTERNAL_TRACE_EVENT_UID(profileScope); \
-    if (INTERNAL_TRACE_EVENT_UID(catstatic)->enabled) { \
-      base::debug::TraceLog::GetInstance()->AddTraceEvent( \
+    if (*INTERNAL_TRACE_EVENT_UID(catstatic)) { \
+      TRACE_EVENT_NAMESPACE::internal::AddTraceEvent( \
           TRACE_EVENT_PHASE_BEGIN, \
           INTERNAL_TRACE_EVENT_UID(catstatic), \
-          name, 0, arg1_name, arg1_val, arg2_name, arg2_val, -1, 0, \
-          TRACE_EVENT_FLAG_NONE); \
+          name, 0, TRACE_EVENT_FLAG_NONE, ##__VA_ARGS__); \
       INTERNAL_TRACE_EVENT_UID(profileScope).Initialize( \
           INTERNAL_TRACE_EVENT_UID(catstatic), name); \
     }
 
 // Implementation detail: internal macro to create static category and add begin
 // event if the category is enabled. Also adds the end event when the scope
 // ends. If the elapsed time is < threshold time, the begin/end pair is erased.
 #define INTERNAL_TRACE_EVENT_ADD_SCOPED_IF_LONGER_THAN(threshold, \
-      category, name, arg1_name, arg1_val, arg2_name, arg2_val) \
+        category, name, ...) \
     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category); \
-    base::debug::internal::TraceEndOnScopeCloseThreshold \
+    TRACE_EVENT_NAMESPACE::internal::TraceEndOnScopeCloseThreshold \
         INTERNAL_TRACE_EVENT_UID(profileScope); \
-    if (INTERNAL_TRACE_EVENT_UID(catstatic)->enabled) { \
+    if (*INTERNAL_TRACE_EVENT_UID(catstatic)) { \
       int INTERNAL_TRACE_EVENT_UID(begin_event_id) = \
-        base::debug::TraceLog::GetInstance()->AddTraceEvent( \
+        TRACE_EVENT_NAMESPACE::internal::AddTraceEvent( \
             TRACE_EVENT_PHASE_BEGIN, \
             INTERNAL_TRACE_EVENT_UID(catstatic), \
-            name, 0, arg1_name, arg1_val, arg2_name, arg2_val, -1, 0, \
-            TRACE_EVENT_FLAG_NONE); \
+            name, 0, TRACE_EVENT_FLAG_NONE, ##__VA_ARGS__); \
       INTERNAL_TRACE_EVENT_UID(profileScope).Initialize( \
           INTERNAL_TRACE_EVENT_UID(catstatic), name, \
           INTERNAL_TRACE_EVENT_UID(begin_event_id), threshold); \
     }
 
 // Implementation detail: internal macro to create static category and add
 // event if the category is enabled.
-#define INTERNAL_TRACE_EVENT_ADD_WITH_ID(phase, category, name, id, \
-      arg1_name, arg1_val, arg2_name, arg2_val, flags) \
+#define INTERNAL_TRACE_EVENT_ADD_WITH_ID(phase, category, name, id, flags, \
+                                         ...) \
     INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category); \
-    if (INTERNAL_TRACE_EVENT_UID(catstatic)->enabled) { \
-      base::debug::TraceLog::GetInstance()->AddTraceEvent( \
+    if (*INTERNAL_TRACE_EVENT_UID(catstatic)) { \
+      TRACE_EVENT_NAMESPACE::internal::AddTraceEvent( \
           phase, INTERNAL_TRACE_EVENT_UID(catstatic), \
-          name, id, arg1_name, arg1_val, arg2_name, arg2_val, -1, 0, flags); \
+          name, TRACE_EVENT_NAMESPACE::TraceID(id).data(), flags, \
+          ##__VA_ARGS__); \
     }
 
 template <typename Type>
 struct StaticMemorySingletonTraits;
 
 namespace base {
 
 class RefCountedString;
 
 namespace debug {
 
-// Categories allow enabling/disabling of streams of trace events
-struct TraceCategory {
-  const char* name;
-  volatile bool enabled;
-};
-
-const size_t kTraceMaxNumArgs = 2;
+const int kTraceMaxNumArgs = 2;
 
 // Phase indicates the nature of an event entry. E.g. part of a begin/end pair.
-typedef char TraceEventPhase;
-#define TRACE_EVENT_PHASE_BEGIN     (base::debug::TraceEventPhase('B'))
-#define TRACE_EVENT_PHASE_END       (base::debug::TraceEventPhase('E'))
-#define TRACE_EVENT_PHASE_INSTANT   (base::debug::TraceEventPhase('I'))
-#define TRACE_EVENT_PHASE_START     (base::debug::TraceEventPhase('S'))
-#define TRACE_EVENT_PHASE_FINISH    (base::debug::TraceEventPhase('F'))
-#define TRACE_EVENT_PHASE_METADATA  (base::debug::TraceEventPhase('M'))
-#define TRACE_EVENT_PHASE_COUNTER   (base::debug::TraceEventPhase('C'))
+// New types can be added, but existing values must never change.

[nduca, 2012/01/11 21:49:11]

Might want to clarify them 'must never change'. I think you mean something like
"downstream versions of trace_event reliy on these, so if you change them, you
need to very carefully update downstream as well."

[jbates, 2012/01/11 22:44:35]

I guess I don't see any reason to discuss changing existing values, because if
we've designed this API properly, changing values should never be necessary
(analogous to OpenGL values). Is there a less confusing way to write "existing
values must never change"?

[jbates, 2012/01/11 23:52:44]

Done. Ignore my previous comment, I was confused about whether we were talking
about chromium versions of this comment or third_party versions.

+#define TRACE_EVENT_PHASE_BEGIN    ('B')
+#define TRACE_EVENT_PHASE_END      ('E')
+#define TRACE_EVENT_PHASE_INSTANT  ('I')
+#define TRACE_EVENT_PHASE_START    ('S')
+#define TRACE_EVENT_PHASE_FINISH   ('F')
+#define TRACE_EVENT_PHASE_METADATA ('M')
+#define TRACE_EVENT_PHASE_COUNTER  ('C')
 
-typedef uint8 TraceEventFlags;
-#define TRACE_EVENT_FLAG_NONE        (base::debug::TraceEventFlags(0))
-#define TRACE_EVENT_FLAG_COPY        (base::debug::TraceEventFlags(1<<0))
-#define TRACE_EVENT_FLAG_HAS_ID      (base::debug::TraceEventFlags(1<<1))
+// New types can be added, but existing values must never change.
+#define TRACE_EVENT_FLAG_NONE        (uint8(0))
+#define TRACE_EVENT_FLAG_COPY        (uint8(1<<0))
+#define TRACE_EVENT_FLAG_HAS_ID      (uint8(1<<1))
 
-// Simple union of values. This is much lighter weight than base::Value, which
-// requires dynamic allocation and a vtable. To keep the trace runtime overhead
-// low, we want constant size storage here.
-class BASE_EXPORT TraceValue {
- public:
-  enum Type {
-    TRACE_TYPE_UNDEFINED,
-    TRACE_TYPE_BOOL,
-    TRACE_TYPE_UINT,
-    TRACE_TYPE_INT,
-    TRACE_TYPE_DOUBLE,
-    TRACE_TYPE_POINTER,
-    TRACE_TYPE_STRING,
-    TRACE_TYPE_STATIC_STRING
-  };
-
-  TraceValue() : type_(TRACE_TYPE_UNDEFINED) {
-    value_.as_uint = 0ull;
-  }
-  TraceValue(bool rhs) : type_(TRACE_TYPE_BOOL) {
-    value_.as_bool = rhs;
-  }
-  TraceValue(uint64 rhs) : type_(TRACE_TYPE_UINT) {
-    value_.as_uint = rhs;
-  }
-  TraceValue(uint32 rhs) : type_(TRACE_TYPE_UINT) {
-    value_.as_uint = rhs;
-  }
-  TraceValue(uint16 rhs) : type_(TRACE_TYPE_UINT) {
-    value_.as_uint = rhs;
-  }
-  TraceValue(uint8 rhs) : type_(TRACE_TYPE_UINT) {
-    value_.as_uint = rhs;
-  }
-  TraceValue(int64 rhs) : type_(TRACE_TYPE_INT) {
-    value_.as_int = rhs;
-  }
-  TraceValue(int32 rhs) : type_(TRACE_TYPE_INT) {
-    value_.as_int = rhs;
-  }
-  TraceValue(int16 rhs) : type_(TRACE_TYPE_INT) {
-    value_.as_int = rhs;
-  }
-  TraceValue(int8 rhs) : type_(TRACE_TYPE_INT) {
-    value_.as_int = rhs;
-  }
-  TraceValue(double rhs) : type_(TRACE_TYPE_DOUBLE) {
-    value_.as_double = rhs;
-  }
-  TraceValue(const void* rhs) : type_(TRACE_TYPE_POINTER) {
-    value_.as_pointer = rhs;
-  }
-  TraceValue(const std::string& rhs) : type_(TRACE_TYPE_STRING) {
-    value_.as_string = rhs.c_str();
-  }
-  TraceValue(const char* rhs) : type_(TRACE_TYPE_STATIC_STRING) {
-    value_.as_string = rhs;
-  }
-
-  static TraceValue StringWithCopy(const char* rhs) {
-    TraceValue value(rhs);
-    if (rhs)
-      value.type_ = TRACE_TYPE_STRING;
-    return value;
-  }
-
-  static TraceValue ForceCopy(const TraceValue& rhs) {
-    TraceValue value(rhs);
-    if (value.type_ == TRACE_TYPE_STATIC_STRING && value.as_string())
-      value.type_ = TRACE_TYPE_STRING;
-    return value;
-  }
-
-  bool is_string() const {
-    return type_ == TRACE_TYPE_STRING || type_ == TRACE_TYPE_STATIC_STRING;
-  }
-
-  void AppendAsJSON(std::string* out) const;
-
-  Type type() const {
-    return type_;
-  }
-  uint64 as_uint() const {
-    DCHECK_EQ(TRACE_TYPE_UINT, type_);
-    return value_.as_uint;
-  }
-  bool as_bool() const {
-    DCHECK_EQ(TRACE_TYPE_BOOL, type_);
-    return value_.as_bool;
-  }
-  int64 as_int() const {
-    DCHECK_EQ(TRACE_TYPE_INT, type_);
-    return value_.as_int;
-  }
-  double as_double() const {
-    DCHECK_EQ(TRACE_TYPE_DOUBLE, type_);
-    return value_.as_double;
-  }
-  const void* as_pointer() const {
-    DCHECK_EQ(TRACE_TYPE_POINTER, type_);
-    return value_.as_pointer;
-  }
-  const char* as_string() const {
-    DCHECK(is_string());
-    return value_.as_string;
-  }
-  const char** as_assignable_string() {
-    DCHECK_EQ(TRACE_TYPE_STRING, type_);
-    return &value_.as_string;
-  }
-
- private:
-  union Value {
-    bool as_bool;
-    uint64 as_uint;
-    int64 as_int;
-    double as_double;
-    const void* as_pointer;
-    const char* as_string;
-  };
-
-  Type type_;
-  Value value_;
-};
+// Type values for identifying types in the TraceValue union.
+// New types can be added, but existing values must never change.
+#define TRACE_VALUE_TYPE_BOOL         (uint8(1))
+#define TRACE_VALUE_TYPE_UINT         (uint8(2))
+#define TRACE_VALUE_TYPE_INT          (uint8(3))
+#define TRACE_VALUE_TYPE_DOUBLE       (uint8(4))
+#define TRACE_VALUE_TYPE_POINTER      (uint8(5))
+#define TRACE_VALUE_TYPE_STRING       (uint8(6))
+#define TRACE_VALUE_TYPE_COPY_STRING  (uint8(7))
 
 // TraceID encapsulates an ID that can either be an integer or pointer. Pointers
 // are mangled with the Process ID so that they are unlikely to collide when the
 // same pointer is used on different processes.
 class BASE_EXPORT TraceID {
  public:
   TraceID() : data_(0u) {}
   TraceID(void* rhs);
   TraceID(unsigned long long rhs) : data_(static_cast<uint64>(rhs)) {}
   TraceID(unsigned long rhs) : data_(static_cast<uint64>(rhs)) {}
   TraceID(unsigned int rhs) : data_(static_cast<uint64>(rhs)) {}
   TraceID(long long rhs) : data_(static_cast<uint64>(rhs)) {}
   TraceID(long rhs) : data_(static_cast<uint64>(rhs)) {}
   TraceID(int rhs) : data_(static_cast<uint64>(rhs)) {}
 
   uint64 data() const { return data_; }
 
  private:
   uint64 data_;
 };
 
 // Output records are "Events" and can be obtained via the
 // OutputCallback whenever the tracing system decides to flush. This
 // can happen at any time, on any thread, or you can programatically
 // force it to happen.
 class BASE_EXPORT TraceEvent {
  public:
+  union TraceValue {
+    bool as_bool;
+    uint64 as_uint;
+    int64 as_int;
+    double as_double;
+    const void* as_pointer;
+    const char* as_string;

[nduca, 2012/01/11 21:49:11]

No clue what goog style says, so I'll assume as_x is what le-style-gods-decree?

[jbates, 2012/01/11 22:44:35]

The TraceValue union was defined this way already, but let me know if there's a
style problem.

+  };
+
   TraceEvent();
   TraceEvent(int thread_id,
              TimeTicks timestamp,
-             TraceEventPhase phase,
-             const TraceCategory* category,
+             char phase,
+             const volatile bool* category_enabled,

[joth, 2012/01/12 16:14:46]

I think you can achieve your goal by just putting volatile on the pointer
through which the bool accessed (up on line 509).
i.e. No need to permeate it throughout the file. (with reference to the thread I
linked above, this is equivalent to hiding its use inside atomic_read() rather
than putting it on the typedef itself)

[jbates, 2012/01/12 19:41:29]

Just getting rid of volatile completely, as it doesn't seem worth the perf
tradeoff as you mentioned.

              const char* name,
-             TraceID id,
-             const char* arg1_name, const TraceValue& arg1_val,
-             const char* arg2_name, const TraceValue& arg2_val,
-             TraceEventFlags flags);
+             uint64 id,
+             int num_args,
+             const char** arg_names,
+             const uint8* arg_types,
+             const uint64* arg_values,
+             uint8 flags);
   ~TraceEvent();
 
-  static char GetPhaseChar(TraceEventPhase phase) {
-    return static_cast<char>(phase);
-  }
-
-  static TraceEventPhase GetPhase(const char* phase) {
-    return static_cast<TraceEventPhase>(*phase);
-  }
-
   // Serialize event data to JSON
   static void AppendEventsAsJSON(const std::vector<TraceEvent>& events,
                                  size_t start,
                                  size_t count,
                                  std::string* out);
   void AppendAsJSON(std::string* out) const;
 
   TimeTicks timestamp() const { return timestamp_; }
 
   // Exposed for unittesting:
 
   const base::RefCountedString* parameter_copy_storage() const {
     return parameter_copy_storage_.get();
   }
 
   const char* name() const { return name_; }
 
  private:
   // Note: these are ordered by size (largest first) for optimal packing.
+  // 64-bit types:

[nduca, 2012/01/11 21:49:11]

not sure we need these comments, but if you feel they're value adds, then at
least make them stentences. // These are all 64 bit types.

[jbates, 2012/01/11 22:44:35]

Does seem redundant with the first comment -- will remove them.

[jbates, 2012/01/11 23:52:44]

Done.

   TimeTicks timestamp_;
   // id_ can be used to store phase-specific data.
-  TraceID id_;
+  uint64 id_;
   TraceValue arg_values_[kTraceMaxNumArgs];
+  // Pointers:
   const char* arg_names_[kTraceMaxNumArgs];
-  const TraceCategory* category_;
+  const volatile bool* category_enabled_;
   const char* name_;
   scoped_refptr<base::RefCountedString> parameter_copy_storage_;
+  // Ints:
   int thread_id_;
-  TraceEventPhase phase_;
-  TraceEventFlags flags_;
+  // Bytes:
+  char phase_;
+  uint8 flags_;
+  uint8 arg_types_[kTraceMaxNumArgs];
 };
 
 
 // TraceResultBuffer collects and converts trace fragments returned by TraceLog
 // to JSON output.
 class BASE_EXPORT TraceResultBuffer {
  public:
   typedef base::Callback<void(const std::string&)> OutputCallback;
 
   // If you don't need to stream JSON chunks out efficiently, and just want to
@@ skipping 89 matching lines @@
   // The trace buffer does not flush dynamically, so when it fills up,
   // subsequent trace events will be dropped. This callback is generated when
   // the trace buffer is full. The callback must be thread safe.
   typedef base::Callback<void(void)> BufferFullCallback;
   void SetBufferFullCallback(const BufferFullCallback& cb);
 
   // Flushes all logged data to the callback.
   void Flush();
 
   // Called by TRACE_EVENT* macros, don't call this directly.
-  static const TraceCategory* GetCategory(const char* name);
+  static const volatile bool* GetCategoryEnabled(const char* name);
+  static const char* GetCategoryName(const volatile bool* category_enabled);
 
   // Called by TRACE_EVENT* macros, don't call this directly.
   // Returns the index in the internal vector of the event if it was added, or
   //         -1 if the event was not added.
   // On end events, the return value of the begin event can be specified along
   // with a threshold in microseconds. If the elapsed time between begin and end
   // is less than the threshold, the begin/end event pair is dropped.
   // If |copy| is set, |name|, |arg_name1| and |arg_name2| will be deep copied
   // into the event; see "Memory scoping note" and TRACE_EVENT_COPY_XXX above.
-  int AddTraceEvent(TraceEventPhase phase,
-                    const TraceCategory* category,
+  int AddTraceEvent(char phase,
+                    const volatile bool* category_enabled,
                     const char* name,
-                    TraceID id,
-                    const char* arg1_name, TraceValue arg1_val,
-                    const char* arg2_name, TraceValue arg2_val,
+                    uint64 id,
+                    int num_args,
+                    const char** arg_names,
+                    const uint8* arg_types,
+                    const uint64* arg_values,
                     int threshold_begin_id,
                     int64 threshold,
-                    TraceEventFlags flags);
-  static void AddTraceEventEtw(TraceEventPhase phase,
+                    uint8 flags);
+  static void AddTraceEventEtw(char phase,
                                const char* name,
                                const void* id,
                                const char* extra);
-  static void AddTraceEventEtw(TraceEventPhase phase,
+  static void AddTraceEventEtw(char phase,
                                const char* name,
                                const void* id,
                                const std::string& extra);
 
   // A wrapper around AddTraceEvent used by TRACE_COUNTERx macros
   // that allows only integer values for the counters.
-  int AddCounterEvent(const TraceCategory* category,
-                      const char* name,
-                      TraceID id,
-                      const char* arg1_name, int32 arg1_val,
-                      const char* arg2_name, int32 arg2_val,
-                      TraceEventFlags flags);
+  void AddCounterEvent(const volatile bool* category_enabled,
+                       const char* name,
+                       uint64 id,
+                       const char* arg1_name, int32 arg1_val,
+                       const char* arg2_name, int32 arg2_val,
+                       uint8 flags);
 
-  // Mangle |id| with a hash based on the process ID so that if |id| occurs on
+  // Mangle |ptr| with a hash based on the process ID so that if |ptr| occurs on
   // more than one process, it will not collide.
-  uint64 GetIntraProcessID(uint64 id) const { return id ^ process_id_hash_; }
+  uint64 GetInterProcessID(void* ptr) const {
+    return static_cast<uint64>(reinterpret_cast<uintptr_t>(ptr)) ^
+           process_id_hash_;
+  }
 
   int process_id() const { return process_id_; }
 
   // Exposed for unittesting:
 
   // Allows deleting our singleton instance.
   static void DeleteForTesting();
 
   // Allows resurrecting our singleton instance post-AtExit processing.
   static void Resurrect();
 
   // Allow tests to inspect TraceEvents.
   size_t GetEventsSize() const { return logged_events_.size(); }
   const TraceEvent& GetEventAt(size_t index) const {
     DCHECK(index < logged_events_.size());
     return logged_events_[index];
   }
 
   void SetProcessID(int process_id);
 
  private:
   // This allows constructor and destructor to be private and usable only
   // by the Singleton class.
   friend struct StaticMemorySingletonTraits<TraceLog>;
 
   TraceLog();
   ~TraceLog();
-  const TraceCategory* GetCategoryInternal(const char* name);
+  const volatile bool* GetCategoryEnabledInternal(const char* name);
   void AddThreadNameMetadataEvents();
   void AddClockSyncMetadataEvents();
 
   // TODO(nduca): switch to per-thread trace buffers to reduce thread
   // synchronization.
   Lock lock_;
   bool enabled_;
   OutputCallback output_callback_;
   BufferFullCallback buffer_full_callback_;
   std::vector<TraceEvent> logged_events_;
   std::vector<std::string> included_categories_;
   std::vector<std::string> excluded_categories_;
 
   base::hash_map<int, std::string> thread_names_;
 
   // XORed with TraceID to make it unlikely to collide with other processes.
   uint64 process_id_hash_;
 
   int process_id_;
 
   DISALLOW_COPY_AND_ASSIGN(TraceLog);
 };
 
 namespace internal {
 
+// Simple union to store various types as uint64.
+union TraceValueUnion {
+  bool as_bool;
+  uint64 as_uint;
+  int64 as_int;
+  double as_double;
+  const void* as_pointer;
+  const char* as_string;
+};
+
+// Simple container for const char* that should be copied instead of retained.
+class TraceStringWithCopy {
+ public:
+  explicit TraceStringWithCopy(const char* str) : str_(str) {}
+  operator const char* () const { return str_; }
+ private:
+  const char* str_;
+};
+
+// Define SetTraceValue for each allowed type. It stores the type and
+// value in the return arguments. This allows this API to avoid declaring any
+// structures so that it is portable to third_party libraries.
+#define INTERNAL_DECLARE_SET_TRACE_VALUE(actual_type, \

[jar (doing other things), 2012/01/12 02:20:03]

I'm not a template master at all... but couldn't this be done better with
templates, and avoid macros??  I don't see the magic that macros bring to the
table here.

[jbates, 2012/01/12 19:41:29]

In this case we need full control over the supported types, so templates would
be too flexible. I did try to get rid of the macro anyway, but it results in a
lot of code duplication, so the macros seems to be the lesser of two evils. I'm
fine either way though, let me know if you'd prefer the functions typed out.

+                                         union_member, \
+                                         value_type_id) \
+    static inline void SetTraceValue(actual_type arg, \
+                                     uint8* type, \
+                                     uint64* value) { \
+      TraceValueUnion type_value; \
+      type_value.union_member = arg; \
+      *type = value_type_id; \
+      *value = type_value.as_uint; \
+    }
+
+INTERNAL_DECLARE_SET_TRACE_VALUE(bool, as_bool, TRACE_VALUE_TYPE_BOOL)
+INTERNAL_DECLARE_SET_TRACE_VALUE(uint64, as_uint, TRACE_VALUE_TYPE_UINT)
+INTERNAL_DECLARE_SET_TRACE_VALUE(uint32, as_uint, TRACE_VALUE_TYPE_UINT)
+INTERNAL_DECLARE_SET_TRACE_VALUE(uint16, as_uint, TRACE_VALUE_TYPE_UINT)
+INTERNAL_DECLARE_SET_TRACE_VALUE(uint8, as_uint, TRACE_VALUE_TYPE_UINT)
+INTERNAL_DECLARE_SET_TRACE_VALUE(int64, as_int, TRACE_VALUE_TYPE_INT)
+INTERNAL_DECLARE_SET_TRACE_VALUE(int32, as_int, TRACE_VALUE_TYPE_INT)
+INTERNAL_DECLARE_SET_TRACE_VALUE(int16, as_int, TRACE_VALUE_TYPE_INT)
+INTERNAL_DECLARE_SET_TRACE_VALUE(int8, as_int, TRACE_VALUE_TYPE_INT)
+INTERNAL_DECLARE_SET_TRACE_VALUE(double, as_double, TRACE_VALUE_TYPE_DOUBLE)
+INTERNAL_DECLARE_SET_TRACE_VALUE(const void*, as_pointer,
+                                 TRACE_VALUE_TYPE_POINTER)
+INTERNAL_DECLARE_SET_TRACE_VALUE(const char*, as_string,
+                                 TRACE_VALUE_TYPE_STRING)
+INTERNAL_DECLARE_SET_TRACE_VALUE(const TraceStringWithCopy&, as_string,
+                                 TRACE_VALUE_TYPE_COPY_STRING)
+

[jar (doing other things), 2012/01/12 02:20:03]

I you really want to use a macro... you may as well undef it here.

[jbates, 2012/01/12 19:41:29]

Done.

+// std::string version of SetTraceValue so that trace arguments can be strings.
+static inline void SetTraceValue(const std::string& arg,
+                                 uint8* type,
+                                 uint64* value) {
+  TraceValueUnion type_value;
+  type_value.as_string = arg.c_str();
+  *type = TRACE_VALUE_TYPE_COPY_STRING;
+  *value = type_value.as_uint;
+}
+
+// These AddTraceEvent template functions are defined here instead of in the
+// macro, because the arg_values could be temporary objects, such as
+// std::string. In order to store pointers to the internal c_str and pass
+// through to the tracing API, the arg_values must live throughout
+// these procedures.
+
+static inline int AddTraceEvent(char phase,
+                                const volatile bool* category_enabled,
+                                const char* name,
+                                uint64 id,
+                                uint8 flags) {
+  return TRACE_EVENT_API_ADD_TRACE_EVENT(
+      phase, category_enabled, name, id,
+      0, NULL, NULL, NULL,
+      -1, 0, flags);
+}
+
+template<class ARG1_TYPE>
+static inline int AddTraceEvent(char phase,
+                                const volatile bool* category_enabled,
+                                const char* name,
+                                uint64 id,
+                                uint8 flags,
+                                const char* arg1_name,
+                                const ARG1_TYPE& arg1_val) {
+  const int num_args = 1;
+  uint8 arg_types[1];
+  uint64 arg_values[1];
+  SetTraceValue(arg1_val, &arg_types[0], &arg_values[0]);
+  return TRACE_EVENT_API_ADD_TRACE_EVENT(
+      phase, category_enabled, name, id,
+      num_args, &arg1_name, arg_types, arg_values,
+      -1, 0, flags);
+}
+
+template<class ARG1_TYPE, class ARG2_TYPE>
+static inline int AddTraceEvent(char phase,
+                                const volatile bool* category_enabled,
+                                const char* name,
+                                uint64 id,
+                                uint8 flags,
+                                const char* arg1_name,
+                                const ARG1_TYPE& arg1_val,
+                                const char* arg2_name,
+                                const ARG2_TYPE& arg2_val) {
+  const int num_args = 2;
+  const char* arg_names[2] = { arg1_name, arg2_name };
+  uint8 arg_types[2];
+  uint64 arg_values[2];
+  SetTraceValue(arg1_val, &arg_types[0], &arg_values[0]);
+  SetTraceValue(arg2_val, &arg_types[1], &arg_values[1]);
+  return TRACE_EVENT_API_ADD_TRACE_EVENT(
+      phase, category_enabled, name, id,
+      num_args, arg_names, arg_types, arg_values,
+      -1, 0, flags);
+}
+
 // Used by TRACE_EVENTx macro. Do not use directly.
 class BASE_EXPORT TraceEndOnScopeClose {
  public:
   // Note: members of data_ intentionally left uninitialized. See Initialize.
   TraceEndOnScopeClose() : p_data_(NULL) {}
   ~TraceEndOnScopeClose() {
     if (p_data_)
       AddEventIfEnabled();
   }
 
-  void Initialize(const TraceCategory* category,
+  void Initialize(const volatile bool* category_enabled,
                   const char* name);
 
  private:
   // Add the end event if the category is still enabled.
   void AddEventIfEnabled();
 
   // This Data struct workaround is to avoid initializing all the members
   // in Data during construction of this object, since this object is always
   // constructed, even when tracing is disabled. If the members of Data were
   // members of this class instead, compiler warnings occur about potential
   // uninitialized accesses.
   struct Data {
-    const TraceCategory* category;
+    const volatile bool* category_enabled;
     const char* name;
   };
   Data* p_data_;
   Data data_;
 };
 
 // Used by TRACE_EVENTx macro. Do not use directly.
 class BASE_EXPORT TraceEndOnScopeCloseThreshold {
  public:
   // Note: members of data_ intentionally left uninitialized. See Initialize.
   TraceEndOnScopeCloseThreshold() : p_data_(NULL) {}
   ~TraceEndOnScopeCloseThreshold() {
     if (p_data_)
       AddEventIfEnabled();
   }
 
   // Called by macros only when tracing is enabled at the point when the begin
   // event is added.
-  void Initialize(const TraceCategory* category,
+  void Initialize(const volatile bool* category_enabled,
                   const char* name,
                   int threshold_begin_id,
                   int64 threshold);
 
  private:
   // Add the end event if the category is still enabled.
   void AddEventIfEnabled();
 
   // This Data struct workaround is to avoid initializing all the members
   // in Data during construction of this object, since this object is always
   // constructed, even when tracing is disabled. If the members of Data were
   // members of this class instead, compiler warnings occur about potential
   // uninitialized accesses.
   struct Data {
     int64 threshold;
-    const TraceCategory* category;
+    const volatile bool* category_enabled;
     const char* name;
     int threshold_begin_id;
   };
   Data* p_data_;
   Data data_;
 };
 
 }  // namespace internal
 
 }  // namespace debug
 }  // namespace base
 
 #endif  // BASE_DEBUG_TRACE_EVENT_H_