Unflake CPU profiler tests.

test/cctest/profiler-extension.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 19 matching lines @@
 #include "src/base/logging.h"
 #include "test/cctest/profiler-extension.h"
 
 namespace v8 {
 namespace internal {
 
 
 v8::CpuProfile* ProfilerExtension::last_profile = NULL;
 const char* ProfilerExtension::kSource =
     "native function startProfiling();"
-    "native function stopProfiling();";
+    "native function stopProfiling();"
+    "native function collectSample();";
+
+namespace {
+v8::Local<v8::String> v8_str(v8::Isolate* isolate, const char* string) {

[yurys, 2016/02/05 17:33:04]

Please use the one defined in v8/test/cctest/cctest.h instead.

[alph, 2016/02/05 17:52:26]

Done that, just forgot to upload. ;-)

+  return v8::String::NewFromUtf8(isolate, string, v8::NewStringType::kNormal)
+      .ToLocalChecked();
+}
+}  // namespace
 
 v8::Local<v8::FunctionTemplate> ProfilerExtension::GetNativeFunctionTemplate(
     v8::Isolate* isolate, v8::Local<v8::String> name) {
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
-  if (name->Equals(context, v8::String::NewFromUtf8(isolate, "startProfiling",
-                                                    v8::NewStringType::kNormal)
-                                .ToLocalChecked())
-          .FromJust()) {
+  if (name->Equals(context, v8_str(isolate, "startProfiling")).FromJust()) {
     return v8::FunctionTemplate::New(isolate,
                                      ProfilerExtension::StartProfiling);
-  } else if (name->Equals(context,
-                          v8::String::NewFromUtf8(isolate, "stopProfiling",
-                                                  v8::NewStringType::kNormal)
-                              .ToLocalChecked())
-                 .FromJust()) {
-    return v8::FunctionTemplate::New(isolate,
-                                     ProfilerExtension::StopProfiling);
-  } else {
-    CHECK(false);
-    return v8::Local<v8::FunctionTemplate>();
   }
+  if (name->Equals(context, v8_str(isolate, "stopProfiling")).FromJust()) {
+    return v8::FunctionTemplate::New(isolate, ProfilerExtension::StopProfiling);
+  }
+  if (name->Equals(context, v8_str(isolate, "collectSample")).FromJust()) {
+    return v8::FunctionTemplate::New(isolate, ProfilerExtension::CollectSample);
+  }
+  CHECK(false);
+  return v8::Local<v8::FunctionTemplate>();
 }
 
-
 void ProfilerExtension::StartProfiling(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   last_profile = NULL;
   v8::CpuProfiler* cpu_profiler = args.GetIsolate()->GetCpuProfiler();
   cpu_profiler->StartProfiling((args.Length() > 0)
       ? args[0].As<v8::String>()
       : v8::String::Empty(args.GetIsolate()));
 }
 
-
 void ProfilerExtension::StopProfiling(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::CpuProfiler* cpu_profiler = args.GetIsolate()->GetCpuProfiler();
   last_profile = cpu_profiler->StopProfiling((args.Length() > 0)
       ? args[0].As<v8::String>()
       : v8::String::Empty(args.GetIsolate()));
 }
 
+void ProfilerExtension::CollectSample(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  args.GetIsolate()->GetCpuProfiler()->CollectSample();

[yurys, 2016/02/05 17:33:04]

The whole idea of the tests that now call this native method was to test the
sampler behavior with various stack states. When deepest js function simply
calls native method, we will always have exit frame on top of the stack and no
js frames above it. This is just one deterministic case among those we wanted to
cover. Can we instead feed the sampler with an artificially constructed top
frame (as we discussed a while ago) and probe into different pc positions inside
js functions of interest?

[alph, 2016/02/05 17:52:26]

The tests I'm unflaking are mostly checking the stack collection for various
combinations of stack frames (e.g. js-native-js, call, apply, deep stack, etc).
And yes they can be done deterministically.
The one you're suggesting is another test that I agree is worth adding. However
it doesn't seem to belong to this patch, which intent is to enable important
profiler tests currently disabled.
wdyt?

[alph, 2016/02/05 18:03:33]

Ok, how about I drop the CheckChildrenNames which I believe is the main source
of flakiness, while keep the stochastic sampling as is?

[yurys, 2016/02/05 18:07:33]

Well, the tests like JsNative1JsNative2JsSample are becoming ones always ending
in a native call, i.e.
JsNative1JsNative2JsSampleNative. In that case it can be simplified even further
to be just JsNative1JsNative2, right?
I agree that it is important to find a way to fix and enable those test as
having them skipped adds little value.

+}
+
 }  // namespace internal
 }  // namespace v8