[Tracing] V8 Tracing Controller - Add args and copy support

test/cctest/libplatform/test-tracing.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-#include <stdio.h>
+#include <limits>
 
 #include "include/libplatform/v8-tracing.h"
 #include "src/tracing/trace-event.h"
 #include "test/cctest/cctest.h"
 
 namespace v8 {
 namespace platform {
 namespace tracing {
 
 TEST(TestTraceConfig) {
@@ skipping 120 matching lines @@
         'Y', tracing_controller.GetCategoryGroupEnabled("v8-cat"), "Test1",
         v8::internal::tracing::kGlobalScope, 43, 456, 0, NULL, NULL, NULL, 0,
         55, 66, 110, 55, 77, 88);
     writer->AppendTraceEvent(&trace_object);
     tracing_controller.StopTracing();
   }
 
   std::string trace_str = stream.str();
   std::string expected_trace_str =
       "{\"traceEvents\":[{\"pid\":11,\"tid\":22,\"ts\":100,\"tts\":50,"
-      "\"ph\":\"X\",\"cat\":\"v8-cat\",\"name\":\"Test0\",\"args\":{},"
-      "\"dur\":33,\"tdur\":44},{\"pid\":55,\"tid\":66,\"ts\":110,\"tts\":55,"
-      "\"ph\":\"Y\",\"cat\":\"v8-cat\",\"name\":\"Test1\",\"args\":{},\"dur\":"
-      "77,\"tdur\":88}]}";
+      "\"ph\":\"X\",\"cat\":\"v8-cat\",\"name\":\"Test0\",\"dur\":33,"
+      "\"tdur\":44,\"args\":{}},{\"pid\":55,\"tid\":66,\"ts\":110,\"tts\":55,"
+      "\"ph\":\"Y\",\"cat\":\"v8-cat\",\"name\":\"Test1\",\"dur\":77,"
+      "\"tdur\":88,\"args\":{}}]}";
 
   CHECK_EQ(expected_trace_str, trace_str);
 
   i::V8::SetPlatformForTesting(old_platform);
 }
 
 TEST(TestTracingController) {
   v8::Platform* old_platform = i::V8::GetCurrentPlatform();
   v8::Platform* default_platform = v8::platform::CreateDefaultPlatform();
   i::V8::SetPlatformForTesting(default_platform);
@@ skipping 15 matching lines @@
   TRACE_EVENT0("v8", "v8.Test3");
   tracing_controller.StopTracing();
 
   CHECK_EQ(2, writer->events().size());
   CHECK_EQ(std::string("v8.Test"), writer->events()[0]);
   CHECK_EQ(std::string("v8.Test3"), writer->events()[1]);
 
   i::V8::SetPlatformForTesting(old_platform);
 }
 
+void GetJSONStrings(std::vector<std::string>& ret, std::string str,
+                    std::string param, std::string start_delim,
+                    std::string end_delim) {
+  size_t pos = str.find(param);
+  while (pos != std::string::npos) {
+    size_t start_pos = str.find(start_delim, pos + param.length());
+    size_t end_pos = str.find(end_delim, start_pos + 1);
+    CHECK_NE(start_pos, std::string::npos);
+    CHECK_NE(end_pos, std::string::npos);
+    ret.push_back(str.substr(start_pos + 1, end_pos - start_pos - 1));
+    pos = str.find(param, pos + 1);
+  }
+}
+
+TEST(TestTracingControllerMultipleArgsAndCopy) {
+  std::ostringstream stream;
+  v8::Platform* old_platform = i::V8::GetCurrentPlatform();
+  v8::Platform* default_platform = v8::platform::CreateDefaultPlatform();
+  i::V8::SetPlatformForTesting(default_platform);
+
+  uint64_t aa = 11;
+  unsigned int bb = 22;
+  uint16_t cc = 33;
+  unsigned char dd = 44;
+  int64_t ee = -55;
+  int ff = -66;
+  int16_t gg = -77;
+  signed char hh = -88;
+  bool ii1 = true;
+  bool ii2 = false;
+  double jj1 = 99.0;
+  double jj2 = 1e100;
+  double jj3 = std::numeric_limits<double>::quiet_NaN();
+  double jj4 = std::numeric_limits<double>::infinity();
+  double jj5 = -std::numeric_limits<double>::infinity();
+  void* kk = &aa;
+  const char* ll = "100";
+  std::string mm = "INIT";
+  std::string v8 = "v8";
+
+  // Create a scope for the tracing controller to terminate the trace writer.
+  {
+    TracingController tracing_controller;
+    platform::SetTracingController(default_platform, &tracing_controller);
+    TraceWriter* writer = TraceWriter::CreateJSONTraceWriter(stream);
+
+    TraceBuffer* ring_buffer =
+        TraceBuffer::CreateTraceBufferRingBuffer(1, writer);
+    tracing_controller.Initialize(ring_buffer);
+    TraceConfig* trace_config = new TraceConfig();
+    trace_config->AddIncludedCategory("v8");
+    tracing_controller.StartTracing(trace_config);
+
+    TRACE_EVENT1("v8", "v8.Test.aa", "aa", aa);
+    TRACE_EVENT1("v8", "v8.Test.bb", "bb", bb);
+    TRACE_EVENT1("v8", "v8.Test.cc", "cc", cc);
+    TRACE_EVENT1("v8", "v8.Test.dd", "dd", dd);
+    TRACE_EVENT1("v8", "v8.Test.ee", "ee", ee);
+    TRACE_EVENT1("v8", "v8.Test.ff", "ff", ff);
+    TRACE_EVENT1("v8", "v8.Test.gg", "gg", gg);
+    TRACE_EVENT1("v8", "v8.Test.hh", "hh", hh);
+    TRACE_EVENT1("v8", "v8.Test.ii", "ii1", ii1);
+    TRACE_EVENT1("v8", "v8.Test.ii", "ii2", ii2);
+    TRACE_EVENT1("v8", "v8.Test.jj1", "jj1", jj1);
+    TRACE_EVENT1("v8", "v8.Test.jj2", "jj2", jj2);
+    TRACE_EVENT1("v8", "v8.Test.jj3", "jj3", jj3);
+    TRACE_EVENT1("v8", "v8.Test.jj4", "jj4", jj4);
+    TRACE_EVENT1("v8", "v8.Test.jj5", "jj5", jj5);
+    TRACE_EVENT1("v8", "v8.Test.kk", "kk", kk);
+    TRACE_EVENT1("v8", "v8.Test.ll", "ll", ll);
+    TRACE_EVENT1("v8", "v8.Test.mm", "mm", TRACE_STR_COPY(mm.c_str()));
+
+    TRACE_EVENT2("v8", "v8.Test2.1", "aa", aa, "ll", ll);
+    TRACE_EVENT2("v8", "v8.Test2.2", "mm1", TRACE_STR_COPY(mm.c_str()), "mm2",
+                 TRACE_STR_COPY(mm.c_str()));
+
+    // Check copies are correct.
+    TRACE_EVENT_COPY_INSTANT0(v8.c_str(), mm.c_str(), TRACE_EVENT_SCOPE_THREAD);

[mattloring, 2016/07/29 01:50:50]

Could we additionally add a test that modifies the string argument between when
the trace event is generated and when it is serialized to ensure the copying was
done.

[rskang, 2016/07/29 05:16:26]

Yes my bad, the copy test did not test anything previously. I've added a patch
for it.

+    TRACE_EVENT_COPY_INSTANT1(v8.c_str(), mm.c_str(), TRACE_EVENT_SCOPE_THREAD,
+                              mm.c_str(), mm.c_str());
+    TRACE_EVENT_COPY_INSTANT2(v8.c_str(), mm.c_str(), TRACE_EVENT_SCOPE_THREAD,

[fmeawad, 2016/07/29 14:32:20]

/s/v8.c_str()/"v8"/  
Category has to be a constant string
Although it is constant here, it gives the impression that it is not.
I would hate somebody looking at the test case and thinking they can get away
with something similar in the code but dynamic instead.

Event name does not have the same strict rules, but in some cases it does, so
ditto on that too.

[rskang, 2016/07/29 19:48:13]

My bad, the v8.c_str() should not be there. This is fixed in the new patch.

+                              mm.c_str(), mm.c_str(), mm.c_str(), mm.c_str());
+
+    tracing_controller.StopTracing();
+  }
+
+  std::string trace_str = stream.str();
+
+  std::vector<std::string> all_args, all_names, all_cats;
+  GetJSONStrings(all_args, trace_str, "\"args\"", "{", "}");
+  GetJSONStrings(all_names, trace_str, "\"name\"", "\"", "\"");
+  GetJSONStrings(all_cats, trace_str, "\"cat\"", "\"", "\"");
+
+  mm = "CHANGED";
+  v8 = "v8-changed";
+
+  CHECK_EQ(all_args.size(), 23);
+  CHECK_EQ(all_args[0], "\"aa\":11");
+  CHECK_EQ(all_args[1], "\"bb\":22");
+  CHECK_EQ(all_args[2], "\"cc\":33");
+  CHECK_EQ(all_args[3], "\"dd\":44");
+  CHECK_EQ(all_args[4], "\"ee\":-55");
+  CHECK_EQ(all_args[5], "\"ff\":-66");
+  CHECK_EQ(all_args[6], "\"gg\":-77");
+  CHECK_EQ(all_args[7], "\"hh\":-88");
+  CHECK_EQ(all_args[8], "\"ii1\":true");
+  CHECK_EQ(all_args[9], "\"ii2\":false");
+  CHECK_EQ(all_args[10], "\"jj1\":99.0");
+  CHECK_EQ(all_args[11], "\"jj2\":1e+100");
+  CHECK_EQ(all_args[12], "\"jj3\":\"NaN\"");
+  CHECK_EQ(all_args[13], "\"jj4\":\"Infinity\"");
+  CHECK_EQ(all_args[14], "\"jj5\":\"-Infinity\"");
+  std::ostringstream pointer_stream;
+  pointer_stream << "\"kk\":\"" << &aa << "\"";
+  CHECK_EQ(all_args[15], pointer_stream.str());
+  CHECK_EQ(all_args[16], "\"ll\":\"100\"");
+  CHECK_EQ(all_args[17], "\"mm\":\"INIT\"");
+
+  CHECK_EQ(all_names[18], "v8.Test2.1");
+  CHECK_EQ(all_args[18], "\"aa\":11,\"ll\":\"100\"");
+  CHECK_EQ(all_args[19], "\"mm1\":\"INIT\",\"mm2\":\"INIT\"");
+
+  CHECK_EQ(all_names[20], "INIT");
+  CHECK_EQ(all_cats[20], "v8");
+  CHECK_EQ(all_names[21], "INIT");
+  CHECK_EQ(all_cats[21], "v8");
+  CHECK_EQ(all_args[21], "\"INIT\":\"INIT\"");
+  CHECK_EQ(all_names[22], "INIT");
+  CHECK_EQ(all_cats[22], "v8");
+  CHECK_EQ(all_args[22], "\"INIT\":\"INIT\",\"INIT\":\"INIT\"");
+
+  i::V8::SetPlatformForTesting(old_platform);
+}
+
 }  // namespace tracing
 }  // namespace platform
 }  // namespace v8