Replaced std::string argument storage in gpu_trace_event with faster TraceValue. Second try to work around arm NULL definition bug.

previous review: http://codereview.chromium.org/6877101

BUG=80172
TEST=tbd

Review URL: http://codereview.chromium.org/6894016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@82696 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 215 insertions, 25 deletions

diff --git a/content/renderer/renderer_gl_context.cc b/content/renderer/renderer_gl_context.cc
index 18e0c1d..8d559c4 100644
--- a/content/renderer/renderer_gl_context.cc
+++ b/content/renderer/renderer_gl_context.cc
@@ -376,7 +376,8 @@ bool RendererGLContext::Initialize(bool onscreen,
                                    const int32* attrib_list,
                                    const GURL& active_url) {
   DCHECK(size.width() >= 0 && size.height() >= 0);
-  GPU_TRACE_EVENT0("gpu", "RendererGLContext::Initialize");
+  GPU_TRACE_EVENT2("gpu", "RendererGLContext::Initialize",
+                   "on_screen", onscreen, "num_pixels", size.GetArea());
 
   if (channel_->state() != GpuChannelHost::kConnected)
     return false;
diff --git a/gpu/common/gpu_trace_event.cc b/gpu/common/gpu_trace_event.cc
index 882b0b8..241ca4e 100644
--- a/gpu/common/gpu_trace_event.cc
+++ b/gpu/common/gpu_trace_event.cc
@@ -39,6 +39,84 @@ TraceCategory::~TraceCategory() {
 
 ////////////////////////////////////////////////////////////////////////////////
 //
+// TraceValue
+//
+////////////////////////////////////////////////////////////////////////////////
+
+void TraceValue::Destroy() {
+  if (type_ == TRACE_TYPE_STRING) {
+    free(value_.as_string);
+    value_.as_string = NULL;
+  }
+  type_ = TRACE_TYPE_UNDEFINED;
+  value_.as_uint = 0ull;
+}
+
+TraceValue& TraceValue::operator=(const TraceValue& rhs) {
+  DCHECK(sizeof(value_) == sizeof(uint64));
+  Destroy();
+  type_ = rhs.type_;
+  if (rhs.type_ == TRACE_TYPE_STRING) {
+    value_.as_string = base::strdup(rhs.value_.as_string);
+  } else {
+    // Copy all 64 bits for all other types.
+    value_.as_uint = rhs.value_.as_uint;
+  }
+  return *this;
+}
+
+bool TraceValue::operator==(const TraceValue& rhs) const {
+  if (type_ != rhs.type())
+    return false;
+  if (rhs.type_ == TRACE_TYPE_STRING) {
+    return (strcmp(value_.as_string, rhs.value_.as_string) == 0);
+  } else {
+    // Compare all 64 bits for all other types. Unused bits are set to zero
+    // by the constructors of types that may be less than 64 bits.
+    return (value_.as_uint == rhs.value_.as_uint);
+  }
+}
+
+void TraceValue::AppendAsJSON(std::string* out) const {
+  char temp_string[128];
+  std::string::size_type start_pos;
+  switch (type_) {
+  case TRACE_TYPE_BOOL:
+    *out += as_bool()? "true" : "false";
+    break;
+  case TRACE_TYPE_UINT:
+    base::snprintf(temp_string, sizeof(temp_string), "%llu",
+                   static_cast<unsigned long long>(as_uint()));
+    *out += temp_string;
+    break;
+  case TRACE_TYPE_INT:
+    base::snprintf(temp_string, sizeof(temp_string), "%lld",
+                   static_cast<long long>(as_int()));
+    *out += temp_string;
+    break;
+  case TRACE_TYPE_DOUBLE:
+    base::snprintf(temp_string, sizeof(temp_string), "%f", as_double());
+    *out += temp_string;
+    break;
+  case TRACE_TYPE_POINTER:
+    base::snprintf(temp_string, sizeof(temp_string), "%p", as_pointer());
+    *out += temp_string;
+    break;
+  case TRACE_TYPE_STRING:
+    start_pos = out->size();
+    *out += as_string();
+    // replace " character with '
+    while ((start_pos = out->find_first_of('\"', start_pos)) !=
+           std::string::npos)
+      (*out)[start_pos] = '\'';
+    break;
+  default:
+    break;
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+//
 // TraceEvent
 //
 ////////////////////////////////////////////////////////////////////////////////
@@ -94,23 +172,23 @@ void TraceEvent::AppendAsJSON(std::string* out) const {
 
   const char* phaseStr = GetPhaseStr(phase);
   int64 time_int64 = timestamp.ToInternalValue();
-  long long unsigned int time_llui =
-      static_cast<long long unsigned int>(time_int64);
   StringAppendF(out,
-      "{cat:'%s',pid:%i,tid:%i,ts:0x%llx,ph:'%s',name:'%s',args:{",
+      "{\"cat\":\"%s\",\"pid\":%i,\"tid\":%i,\"ts\":%lld,"
+      "\"ph\":\"%s\",\"name\":\"%s\",\"args\":{",
       category->name(),
       static_cast<int>(processId),
       static_cast<int>(threadId),
-      time_llui,
+      static_cast<long long>(time_int64),
       phaseStr,
       name);
   for (int  i = 0; i < nargs; ++i) {
     if (i > 0)
       *out += ",";
+    *out += "\"";
     *out += argNames[i];
-    *out += ":'";
-    *out += argValues[i];
-    *out += "'";
+    *out += "\":\"";
+    argValues[i].AppendAsJSON(out);
+    *out += "\"";
   }
   *out += "}}";
 }
@@ -213,8 +291,8 @@ void TraceLog::AddTraceEvent(TraceEventPhase phase,
     const char* file, int line,
     TraceCategory* category,
     const char* name,
-    const char* arg1name, const char* arg1val,
-    const char* arg2name, const char* arg2val) {
+    const char* arg1name, TraceValue arg1val,
+    const char* arg2name, TraceValue arg2val) {
   DCHECK(file && name);
 #ifdef USE_UNRELIABLE_NOW
   TimeTicks now = TimeTicks::HighResNow();
@@ -234,9 +312,9 @@ void TraceLog::AddTraceEvent(TraceEventPhase phase,
   event.category = category;
   event.name = name;
   event.argNames[0] = arg1name;
-  event.argValues[0] = arg1name ? arg1val : "";
+  event.argValues[0] = arg1val;
   event.argNames[1] = arg2name;
-  event.argValues[1] = arg2name ? arg2val : "";
+  event.argValues[1] = arg2val;
   COMPILE_ASSERT(TRACE_MAX_NUM_ARGS == 2, TraceEvent_arc_count_out_of_sync);
   if (logged_events_.size() == TRACE_EVENT_BUFFER_SIZE &&
       buffer_full_callback_.get())
diff --git a/gpu/common/gpu_trace_event.h b/gpu/common/gpu_trace_event.h
index baf1392..d71e059 100644
--- a/gpu/common/gpu_trace_event.h
+++ b/gpu/common/gpu_trace_event.h
@@ -72,6 +72,7 @@
 #include "base/time.h"
 #include "base/timer.h"
 #include "base/callback.h"
+#include "base/string_util.h"
 #include <vector>
 
 
@@ -88,9 +89,9 @@
 // scope, with 0, 1 or 2 associated arguments. If the category is not
 // enabled, then this does nothing.
 #define GPU_TRACE_EVENT0(category, name) \
-  GPU_TRACE_EVENT1(category, name, NULL, NULL)
+  GPU_TRACE_EVENT1(category, name, NULL, 0)
 #define GPU_TRACE_EVENT1(category, name, arg1name, arg1val) \
-  GPU_TRACE_EVENT2(category, name, arg1name, arg1val, NULL, NULL)
+  GPU_TRACE_EVENT2(category, name, arg1name, arg1val, NULL, 0)
 #define GPU_TRACE_EVENT2(category, name, arg1name, arg1val, arg2name, arg2val) \
   static gpu::TraceCategory* \
       GPU_TRACE_EVENT_UNIQUE_IDENTIFIER(catstatic) = \
@@ -113,9 +114,9 @@
 // associated arguments. If the category is not enabled, then this
 // does nothing.
 #define GPU_TRACE_EVENT_INSTANT0(category, name) \
-  GPU_TRACE_EVENT_INSTANT1(category, name, NULL, NULL)
+  GPU_TRACE_EVENT_INSTANT1(category, name, NULL, 0)
 #define GPU_TRACE_EVENT_INSTANT1(category, name, arg1name, arg1val) \
-  GPU_TRACE_EVENT_INSTANT2(category, name, arg1name, arg1val, NULL, NULL)
+  GPU_TRACE_EVENT_INSTANT2(category, name, arg1name, arg1val, NULL, 0)
 #define GPU_TRACE_EVENT_INSTANT2(category, name, arg1name, arg1val, \
     arg2name, arg2val) \
   static gpu::TraceCategory* \
@@ -136,9 +137,9 @@
 // associated arguments. If the category is not enabled, then this
 // does nothing.
 #define GPU_TRACE_EVENT_BEGIN0(category, name) \
-  GPU_TRACE_EVENT_BEGIN1(category, name, NULL, NULL)
+  GPU_TRACE_EVENT_BEGIN1(category, name, NULL, 0)
 #define GPU_TRACE_EVENT_BEGIN1(category, name, arg1name, arg1val) \
-  GPU_TRACE_EVENT_BEGIN2(category, name, arg1name, arg1val, NULL, NULL)
+  GPU_TRACE_EVENT_BEGIN2(category, name, arg1name, arg1val, NULL, 0)
 #define GPU_TRACE_EVENT_BEGIN2(category, name, arg1name, arg1val, \
     arg2name, arg2val) \
   static gpu::TraceCategory* \
@@ -168,8 +169,8 @@
         __FILE__, __LINE__, \
         GPU_TRACE_EVENT_UNIQUE_IDENTIFIER(catstatic), \
         name, \
-        NULL, NULL, \
-        NULL, NULL); \
+        NULL, 0, \
+        NULL, 0); \
   }
 
 
@@ -199,6 +200,116 @@ enum TraceEventPhase {
   GPU_TRACE_EVENT_PHASE_INSTANT
 };
 
+// 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 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
+  };
+
+  TraceValue() : type_(TRACE_TYPE_UNDEFINED) {
+    value_.as_uint = 0ull;
+  }
+  TraceValue(bool rhs) : type_(TRACE_TYPE_BOOL) {
+    value_.as_uint = 0ull; // zero all bits
+    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_uint = 0ull; // zero all bits
+    value_.as_pointer = rhs;
+  }
+  explicit TraceValue(const char* rhs) : type_(TRACE_TYPE_STRING) {
+    value_.as_uint = 0ull; // zero all bits
+    value_.as_string = base::strdup(rhs);
+  }
+  TraceValue(const TraceValue& rhs) : type_(TRACE_TYPE_UNDEFINED) {
+    operator=(rhs);
+  }
+  ~TraceValue() {
+    Destroy();
+  }
+
+  TraceValue& operator=(const TraceValue& rhs);
+  bool operator==(const TraceValue& rhs) const;
+  bool operator!=(const TraceValue& rhs) const {
+    return !operator==(rhs);
+  }
+
+  void Destroy();
+
+  void AppendAsJSON(std::string* out) const;
+
+  Type type() const {
+    return type_;
+  }
+  uint64 as_uint() const {
+    return value_.as_uint;
+  }
+  bool as_bool() const {
+    return value_.as_bool;
+  }
+  int64 as_int() const {
+    return value_.as_int;
+  }
+  double as_double() const {
+    return value_.as_double;
+  }
+  const void* as_pointer() const {
+    return value_.as_pointer;
+  }
+  const char* as_string() const {
+    return value_.as_string;
+  }
+
+ private:
+  union Value {
+    bool as_bool;
+    uint64 as_uint;
+    int64 as_int;
+    double as_double;
+    const void* as_pointer;
+    char* as_string;
+  };
+
+  Type type_;
+  Value value_;
+};
+
 // Output records are "Events" and can be obtained via the
 // OutputCallback whenever the logging system decides to flush. This
 // can happen at any time, on any thread, or you can programatically
@@ -220,7 +331,7 @@ struct TraceEvent {
   TraceCategory* category;
   const char* name;
   const char* argNames[TRACE_MAX_NUM_ARGS];
-  std::string argValues[TRACE_MAX_NUM_ARGS];
+  TraceValue argValues[TRACE_MAX_NUM_ARGS];
 };
 
 
@@ -262,8 +373,8 @@ class TraceLog {
       const char* file, int line,
       TraceCategory* category,
       const char* name,
-      const char* arg1name, const char* arg1val,
-      const char* arg2name, const char* arg2val);
+      const char* arg1name, TraceValue arg1val,
+      const char* arg2name, TraceValue arg2val);
 
  private:
   // This allows constructor and destructor to be private and usable only
@@ -306,7 +417,7 @@ class TraceEndOnScopeClose {
           file_, line_,
           category_,
           name_,
-          NULL, NULL, NULL, NULL);
+          NULL, 0, NULL, 0);
   }
 
  private:
diff --git a/webkit/glue/webkitclient_impl.cc b/webkit/glue/webkitclient_impl.cc
index 4e2b10a..b2dd53a 100644
--- a/webkit/glue/webkitclient_impl.cc
+++ b/webkit/glue/webkitclient_impl.cc
@@ -286,7 +286,7 @@ void WebKitClientImpl::traceEventBegin(const char* name, void* id,
                                        const char* extra) {
   TRACE_EVENT_BEGIN(name, id, extra);
   GPU_TRACE_EVENT_BEGIN2("webkit", name,
-                         "id", StringPrintf("%p", id).c_str(),
+                         "id", id,
                          "extra", extra ? extra : "");
 }