Initial GetSample implementation.

include/v8-sampler.h

Index: include/v8-sampler.h
diff --git a/include/v8-sampler.h b/include/v8-sampler.h
new file mode 100644
index 0000000000000000000000000000000000000000..7e9177022e0cbe5aa0c3a5334dafca373e2fa55b
--- /dev/null
+++ b/include/v8-sampler.h
@@ -0,0 +1,60 @@
+// Copyright 2014 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.
+
+#ifndef V8_V8_SAMPLER_H_
+#define V8_V8_SAMPLER_H_
+
+#include "v8.h"
+
+/**
+ * Sampler API for the V8 JavaScript engine.
+ * The API to be consumed by any code which
+ * wants to build a sampling profiler for v8.
+ */
+namespace v8 {
+  /* TODO(gholap): This should go away and struct Sample should
+                   just use const void* instead of Address.
+                   Currently we need it because of implementation details. */
+  typedef unsigned char* Address;
+
+/**
+ * A collected sample contains,
+ * - state       : The state of the VM at the time of collecting the sample.
+ * - stack       : An array of addresses.
+ *                 One address per stack frame.
+ *                 The address is the instruction pointer,
+ *                 pointing to the instruction which led to the
+ *                 creation of the stack frame.
+ *                 (for example, a function call)
+ * - frames_count: Number of stack frames that were captured.
+ *                 That is, stack[frames_count+i] might contain meaningless
+ *                 addresses for any i >= 0.
+ */
+struct Sample {
+  Sample()
+      : frames_count(0) {}
+  static const unsigned kMaxFramesCountLog2 = 8;
+  static const unsigned kMaxFramesCount = (1 << kMaxFramesCountLog2) - 1;
+
+  Address stack[kMaxFramesCount];               // Call stack.
+  unsigned frames_count : kMaxFramesCountLog2;  // Number of captured frames.

[Sven Panne, 2014/08/27 06:13:47]

Don't use a bit field here, a plain unsigned int would be better. Nuke
kMaxFramesCountLog2.

[gholap, 2014/08/27 21:48:08]

Done.

+};
+
+/**
+ * Interface for collecting execution stack samples.
+ */
+class V8_EXPORT Sampler {
+ public:
+  /**
+   * Obtain a sample from the isolate.
+   * Updates the sample pointer with the newly obtained
+   * sampling information.
+   * On success, returns the sample pointer.
+   * On failure, returns NULL.
+   */
+  static Sample* GetSample(Isolate* isolate,

[Benedikt Meurer, 2014/08/27 04:17:23]

Looking at the use in the test, I think we should have GetSample return a bool
instead.

[Sven Panne, 2014/08/27 06:13:47]

I would even go a step further and return void: Setting frames_count to zero is
consistent and easier, I think. Or do we explicitly want to distinguish between
some (undocumented!) error conditions and empty samples? I don't think so...

Furthermore, I don't really like static functions, they are a strong indicator
that the function should really live somewhere else. In our case: Just make it a
member function of Isolate, nuke the Sampler class. Then we can move Sample to
v8.h, too and remove the whole v8-sampler.h.

[gholap, 2014/08/27 21:48:08]

Done.

[gholap, 2014/08/27 21:48:08]

Yes, that would surely be a better way to do it compared to the redundant and
ambiguous use of sample pointer.

As per Sven's suggestion, setting the frame count to zero should also do. I'm
not able to see a case rightaway where that would be a problem, but you are much
more familiar with the code and you might, please let me know in that case...

+                           Sample* sample);
+};
+}  // namespace v8

[Benedikt Meurer, 2014/08/27 04:17:23]

Nit: Add newline after class declaration.

[gholap, 2014/08/27 21:48:08]

Acknowledged.

+#endif  // V8_V8_SAMPLER_H_