Update Xray for PNaCl

native_client_sdk/src/libraries/xray/browser.c

Index: native_client_sdk/src/libraries/xray/browser.c
diff --git a/native_client_sdk/src/libraries/xray/browser.c b/native_client_sdk/src/libraries/xray/browser.c
new file mode 100644
index 0000000000000000000000000000000000000000..be26b3249f6fc1ece0163817921014dde2d70c73
--- /dev/null
+++ b/native_client_sdk/src/libraries/xray/browser.c
@@ -0,0 +1,145 @@
+/* Copyright (c) 2013 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.
+ */
+
+
+/* XRay -- a simple profiler for Native Client */
+

[nfullagar1, 2013/07/17 00:53:19]

wrap this file with #ifndef XRAY_DISABLE_BROWSER_INTEGRATION

[grosse, 2013/07/17 19:37:56]

Done.

+#include <alloca.h>
+#include <assert.h>
+#include <errno.h>
+#include <stdarg.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include "ppapi/c/dev/ppb_trace_event_dev.h"
+#include "xray/xray_priv.h"
+
+
+#if defined(XRAY)
+static PPB_GetInterface get_browser_interface = NULL;
+static PPB_Trace_Event_Dev* trace;
+
+const char* XRayGetName(struct XRaySymbolTable* symbols,
+      struct XRayTraceBufferEntry* e)
+{
+  uint32_t addr = XRAY_EXTRACT_ADDR(e->depth_addr);
+  struct XRaySymbol* symbol = XRaySymbolTableLookup(symbols, addr);
+  return XRaySymbolGetName(symbol);
+}
+
+struct XrayTimestampPair XRayPepperBeginCalibration(void)

[nfullagar1, 2013/07/17 00:53:19]

{ hoisted to end of line above, here and else where

[grosse, 2013/07/17 19:37:56]

Done.

+{
+  struct XrayTimestampPair pair;
+  assert(trace);
+
+  XRayGetTSC(&pair.xray);
+  pair.pepper = trace->Now();
+  return pair;
+}
+
+
+void XRayBrowserTraceReport(struct XRayTraceCapture* capture)
+{
+
+  struct XrayTimestampPair start_time = *XRayGetStartTimestamp(capture);
+  struct XrayTimestampPair end_time = XRayPepperBeginCalibration();

[nfullagar1, 2013/07/17 00:53:19]

A bit confusing above - end_time is set to 'BeginCalibration', but we've already
begun calibration on the first frame.

I'd also again recommend doing this calibration per-frame, some bookkeeping is
already there.  Consider the non-pnacl case, where XRay is using RDTSC.  It
might be safer to scale each frame individually and avoid wrapping the counter. 
(detecting the wrap would be bonus)

+
+  double pdiff = (end_time.pepper - start_time.pepper);
+  double odiff = (end_time.xray - start_time.xray);
+  double scale_a = pdiff/odiff;

[nfullagar1, 2013/07/17 00:53:19]

spaces around binary operator, pdiff / odiff;
(same for below and else where, scale_a * end_time...)

[grosse, 2013/07/17 19:40:01]

Done.

+  double scale_b = ((double)end_time.pepper) - (scale_a*end_time.xray);
+  printf("Calibration: %f %f\n", scale_a, scale_b);
+
+  const void* cat_enabled = trace->GetCategoryEnabled("xray");
+  struct XRaySymbolTable* symbols = XRayGetSymbolTable(capture);
+
+  int head = XRayFrameGetHead(capture);
+  int frame = XRayFrameGetTail(capture);
+  while(frame != head) {
+
+    int start = XRayFrameGetTraceStartIndex(capture, frame);
+    int end = XRayFrameGetTraceEndIndex(capture, frame);
+    int count = XRayFrameGetTraceCount(capture, frame);
+
+    struct XRayTraceBufferEntry** stack_base = XRayMalloc(
+      sizeof(struct XRayTraceBufferEntry*) * (count+1));
+    struct XRayTraceBufferEntry** stack_top = stack_base;
+    *stack_top = NULL;
+
+    int arg_count = 0;
+    const char* arg_names[1];
+    const uint8_t arg_types[1] = {};
+    const uint64_t arg_values[1] = {};
+
+    int i;
+    for(i = start; i != end; i = XRayTraceNextEntry(capture, i)){
+      if (XRayTraceIsAnnotation(capture, i)) {
+        // arg_names[0] = (char*)(XRayTraceGetEntry(capture, i)) + 1;

[nfullagar1, 2013/07/17 00:53:19]

remove dead code?

[grosse, 2013/07/17 19:37:56]

Done.

+        // arg_count = 1;
+        continue;
+      }
+
+
+      struct XRayTraceBufferEntry* e = XRayTraceGetEntry(capture, i);
+      uint32_t depth = XRAY_EXTRACT_DEPTH(e->depth_addr);
+
+      while(*stack_top && e->start_tick >= (*stack_top)->end_tick){
+        struct XRayTraceBufferEntry* e2 = *(stack_top--);

[nfullagar1, 2013/07/17 00:53:19]

something more descriptive than 'e2'

[grosse, 2013/07/17 19:40:01]

Done.

+
+        printf("E %s %llu\n", XRayGetName(symbols, e2), e2->end_tick);
+        trace->AddTraceEventWithThreadIdAndTimestamp(
+            'E', cat_enabled,
+            XRayGetName(symbols, e2),
+            0, 1337,
+            (scale_a * e2->end_tick) + scale_b,
+            0, NULL, NULL, NULL, 0
+        );
+      }
+
+
+      if (arg_count){
+        printf("Sending annotation %s\n", arg_names[0]);
+      }
+
+      printf("B %s %llu\n", XRayGetName(symbols, e), e->start_tick);

[nfullagar1, 2013/07/17 00:53:19]

are the printfs for debugging? imo, for browser trace report, printf should be
minimal (calibration, # frames, # traces total, # annotations)

[grosse, 2013/07/17 19:40:01]

Done.

+      trace->AddTraceEventWithThreadIdAndTimestamp(
+          'B', cat_enabled,
+          XRayGetName(symbols, e),
+          0, 1337,
+          (scale_a * e->start_tick) + scale_b,
+          arg_count, arg_names, arg_types, arg_values, 0
+      );
+
+      arg_count = 0;
+      *(++stack_top) = e;
+    }
+
+    while(*stack_top){
+      struct XRayTraceBufferEntry* e2 = *(stack_top--);
+
+      printf("E %s %llu\n", XRayGetName(symbols, e2), e2->end_tick);
+      trace->AddTraceEventWithThreadIdAndTimestamp(
+          'E', cat_enabled,
+          XRayGetName(symbols, e2),
+          0, 1337,

[nfullagar1, 2013/07/17 00:53:19]

why not get real TID and send it?  (stash in XRayBeginFrame, uploading trace
data to browser might be done from a different thread to avoid janking the
application)  (repeat for other calls to AddTraceEvent...)

+          (scale_a * e2->end_tick) + scale_b,
+          0, NULL, NULL, NULL, 0
+      );
+    }
+
+    frame = XRayFrameGetNext(capture, frame);
+    XRayFree(stack_base);
+  }
+}
+
+void XRayBrowserRegisterInterface(PPB_GetInterface interface) {
+  get_browser_interface = interface;

[nfullagar1, 2013/07/17 00:53:19]

no need to stash get_browser_interface if you only need ppb_trace_interface
below.

[grosse, 2013/07/17 19:40:01]

Done.

+  trace = (PPB_Trace_Event_Dev*)get_browser_interface(

[nfullagar1, 2013/07/17 00:53:19]

ppb_trace_interface

[grosse, 2013/07/17 19:40:01]

Done.

+      PPB_TRACE_EVENT_DEV_INTERFACE);
+}
+
+#endif  // XRAY

[nfullagar1, 2013/07/17 00:53:19]

/* XRAY */
(might as well fix report.c too)

[grosse, 2013/07/17 19:37:56]

Done.