Skip to content

Commit ab70c96

Browse files
joyeecheungtargos
authored andcommittedMar 30, 2019
src: refactor coverage connection
- Refactor the C++ class to be resuable for other types of profiles - Move the try-catch block around coverage collection callback to be inside the callback to silence potential JSON or write errors. - Use Function::Call instead of MakeCallback to call the coverage message callback since it does not actually need async hook handling. This way we no longer needs to disable the async hooks when writing the coverage results. - Renames `lib/internal/coverage-gen/with_profiler.js` to `lib/internal/profiler.js` because it is now the only way to generate coverage. PR-URL: #26513 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Coe <bencoe@gmail.com>
1 parent 63e7cc7 commit ab70c96

11 files changed

+231
-194
lines changed
 

‎lib/internal/bootstrap/pre_execution.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ function setupCoverageHooks(dir) {
7878
const {
7979
writeCoverage,
8080
setCoverageDirectory
81-
} = require('internal/coverage-gen/with_profiler');
81+
} = require('internal/profiler');
8282
setCoverageDirectory(coverageDirectory);
8383
process.on('exit', writeCoverage);
8484
process.reallyExit = (code) => {

‎lib/internal/process/per_thread.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ function wrapProcessMethods(binding) {
158158
function kill(pid, sig) {
159159
var err;
160160
if (process.env.NODE_V8_COVERAGE) {
161-
const { writeCoverage } = require('internal/coverage-gen/with_profiler');
161+
const { writeCoverage } = require('internal/profiler');
162162
writeCoverage();
163163
}
164164

‎lib/internal/coverage-gen/with_profiler.js ‎lib/internal/profiler.js

+6-13
Original file line numberDiff line numberDiff line change
@@ -21,23 +21,16 @@ function writeCoverage() {
2121
}
2222

2323
const target = join(coverageDirectory, filename);
24-
try {
25-
disableAllAsyncHooks();
26-
internalBinding('coverage').end((msg) => {
24+
internalBinding('profiler').endCoverage((msg) => {
25+
try {
2726
const coverageInfo = JSON.parse(msg).result;
2827
if (coverageInfo) {
2928
writeFileSync(target, JSON.stringify(coverageInfo));
3029
}
31-
});
32-
} catch (err) {
33-
console.error(err);
34-
}
35-
}
36-
37-
function disableAllAsyncHooks() {
38-
const { getHookArrays } = require('internal/async_hooks');
39-
const [hooks_array] = getHookArrays();
40-
hooks_array.forEach((hook) => { hook.disable(); });
30+
} catch (err) {
31+
console.error(err);
32+
}
33+
});
4134
}
4235

4336
function setCoverageDirectory(dir) {

‎node.gyp

+1-1
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@
102102
'lib/internal/cluster/worker.js',
103103
'lib/internal/console/constructor.js',
104104
'lib/internal/console/global.js',
105-
'lib/internal/coverage-gen/with_profiler.js',
106105
'lib/internal/crypto/certificate.js',
107106
'lib/internal/crypto/cipher.js',
108107
'lib/internal/crypto/diffiehellman.js',
@@ -170,6 +169,7 @@
170169
'lib/internal/process/worker_thread_only.js',
171170
'lib/internal/process/report.js',
172171
'lib/internal/process/task_queues.js',
172+
'lib/internal/profiler.js',
173173
'lib/internal/querystring.js',
174174
'lib/internal/readline.js',
175175
'lib/internal/repl.js',

‎src/env.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ struct CompileFnEntry {
469469
#define DEBUG_CATEGORY_NAMES(V) \
470470
NODE_ASYNC_PROVIDER_TYPES(V) \
471471
V(INSPECTOR_SERVER) \
472-
V(COVERAGE)
472+
V(INSPECTOR_PROFILER)
473473

474474
enum class DebugCategory {
475475
#define V(name) name,

‎src/inspector/node_inspector.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
'../../src/inspector_io.cc',
4646
'../../src/inspector_agent.h',
4747
'../../src/inspector_io.h',
48-
'../../src/inspector_coverage.cc',
48+
'../../src/inspector_profiler.cc',
4949
'../../src/inspector_js_api.cc',
5050
'../../src/inspector_socket.cc',
5151
'../../src/inspector_socket.h',

‎src/inspector_coverage.cc

-168
This file was deleted.

‎src/inspector_profiler.cc

+214
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
#include "base_object-inl.h"
2+
#include "debug_utils.h"
3+
#include "inspector_agent.h"
4+
#include "node_internals.h"
5+
#include "v8-inspector.h"
6+
7+
namespace node {
8+
namespace profiler {
9+
10+
using v8::Context;
11+
using v8::EscapableHandleScope;
12+
using v8::Function;
13+
using v8::FunctionCallbackInfo;
14+
using v8::HandleScope;
15+
using v8::Isolate;
16+
using v8::Local;
17+
using v8::MaybeLocal;
18+
using v8::NewStringType;
19+
using v8::Object;
20+
using v8::ObjectTemplate;
21+
using v8::String;
22+
using v8::Value;
23+
24+
using v8_inspector::StringBuffer;
25+
using v8_inspector::StringView;
26+
27+
std::unique_ptr<StringBuffer> ToProtocolString(Isolate* isolate,
28+
Local<Value> value) {
29+
TwoByteValue buffer(isolate, value);
30+
return StringBuffer::create(StringView(*buffer, buffer.length()));
31+
}
32+
33+
class V8ProfilerConnection : public BaseObject {
34+
public:
35+
class V8ProfilerSessionDelegate
36+
: public inspector::InspectorSessionDelegate {
37+
public:
38+
explicit V8ProfilerSessionDelegate(V8ProfilerConnection* connection)
39+
: connection_(connection) {}
40+
41+
void SendMessageToFrontend(
42+
const v8_inspector::StringView& message) override {
43+
Environment* env = connection_->env();
44+
45+
Local<Function> fn = connection_->GetMessageCallback();
46+
bool ending = !fn.IsEmpty();
47+
Debug(env,
48+
DebugCategory::INSPECTOR_PROFILER,
49+
"Sending message to frontend, ending = %s\n",
50+
ending ? "true" : "false");
51+
if (!ending) {
52+
return;
53+
}
54+
Isolate* isolate = env->isolate();
55+
56+
HandleScope handle_scope(isolate);
57+
Context::Scope context_scope(env->context());
58+
MaybeLocal<String> v8string =
59+
String::NewFromTwoByte(isolate,
60+
message.characters16(),
61+
NewStringType::kNormal,
62+
message.length());
63+
Local<Value> args[] = {v8string.ToLocalChecked().As<Value>()};
64+
USE(fn->Call(
65+
env->context(), connection_->object(), arraysize(args), args));
66+
}
67+
68+
private:
69+
V8ProfilerConnection* connection_;
70+
};
71+
72+
SET_MEMORY_INFO_NAME(V8ProfilerConnection)
73+
SET_SELF_SIZE(V8ProfilerConnection)
74+
75+
void MemoryInfo(MemoryTracker* tracker) const override {
76+
tracker->TrackFieldWithSize(
77+
"session", sizeof(*session_), "InspectorSession");
78+
}
79+
80+
explicit V8ProfilerConnection(Environment* env, Local<Object> obj)
81+
: BaseObject(env, obj), session_(nullptr) {
82+
inspector::Agent* inspector = env->inspector_agent();
83+
std::unique_ptr<inspector::InspectorSession> session = inspector->Connect(
84+
std::make_unique<V8ProfilerSessionDelegate>(this), false);
85+
session_ = std::move(session);
86+
MakeWeak();
87+
}
88+
89+
void DispatchMessage(Isolate* isolate, Local<String> message) {
90+
session_->Dispatch(ToProtocolString(isolate, message)->string());
91+
}
92+
93+
static MaybeLocal<Object> CreateConnectionObject(Environment* env) {
94+
Isolate* isolate = env->isolate();
95+
Local<Context> context = env->context();
96+
EscapableHandleScope scope(isolate);
97+
98+
Local<ObjectTemplate> t = ObjectTemplate::New(isolate);
99+
t->SetInternalFieldCount(1);
100+
Local<Object> obj;
101+
if (!t->NewInstance(context).ToLocal(&obj)) {
102+
return MaybeLocal<Object>();
103+
}
104+
105+
obj->SetAlignedPointerInInternalField(0, nullptr);
106+
return scope.Escape(obj);
107+
}
108+
109+
void Start() {
110+
SetConnection(object());
111+
StartProfiling();
112+
}
113+
114+
void End(Local<Function> callback) {
115+
SetMessageCallback(callback);
116+
EndProfiling();
117+
}
118+
119+
// Override this to return a JS function that gets called with the message
120+
// sent from the session.
121+
virtual Local<Function> GetMessageCallback() = 0;
122+
virtual void SetMessageCallback(Local<Function> callback) = 0;
123+
// Use DispatchMessage() to dispatch necessary inspector messages
124+
virtual void StartProfiling() = 0;
125+
virtual void EndProfiling() = 0;
126+
virtual void SetConnection(Local<Object> connection) = 0;
127+
128+
private:
129+
std::unique_ptr<inspector::InspectorSession> session_;
130+
};
131+
132+
class V8CoverageConnection : public V8ProfilerConnection {
133+
public:
134+
explicit V8CoverageConnection(Environment* env)
135+
: V8ProfilerConnection(env,
136+
CreateConnectionObject(env).ToLocalChecked()) {}
137+
138+
Local<Function> GetMessageCallback() override {
139+
return env()->on_coverage_message_function();
140+
}
141+
142+
void SetMessageCallback(Local<Function> callback) override {
143+
return env()->set_on_coverage_message_function(callback);
144+
}
145+
146+
static V8ProfilerConnection* GetConnection(Environment* env) {
147+
return Unwrap<V8CoverageConnection>(env->coverage_connection());
148+
}
149+
150+
void SetConnection(Local<Object> obj) override {
151+
env()->set_coverage_connection(obj);
152+
}
153+
154+
void StartProfiling() override {
155+
Debug(env(),
156+
DebugCategory::INSPECTOR_PROFILER,
157+
"Sending Profiler.startPreciseCoverage\n");
158+
Isolate* isolate = env()->isolate();
159+
Local<String> enable = FIXED_ONE_BYTE_STRING(
160+
isolate, "{\"id\": 1, \"method\": \"Profiler.enable\"}");
161+
Local<String> start = FIXED_ONE_BYTE_STRING(
162+
isolate,
163+
"{"
164+
"\"id\": 2,"
165+
"\"method\": \"Profiler.startPreciseCoverage\","
166+
"\"params\": {\"callCount\": true, \"detailed\": true}"
167+
"}");
168+
DispatchMessage(isolate, enable);
169+
DispatchMessage(isolate, start);
170+
}
171+
172+
void EndProfiling() override {
173+
Debug(env(),
174+
DebugCategory::INSPECTOR_PROFILER,
175+
"Sending Profiler.takePreciseCoverage\n");
176+
Isolate* isolate = env()->isolate();
177+
Local<String> end =
178+
FIXED_ONE_BYTE_STRING(isolate,
179+
"{"
180+
"\"id\": 3,"
181+
"\"method\": \"Profiler.takePreciseCoverage\""
182+
"}");
183+
DispatchMessage(isolate, end);
184+
}
185+
186+
private:
187+
std::unique_ptr<inspector::InspectorSession> session_;
188+
};
189+
190+
void StartCoverageCollection(Environment* env) {
191+
V8CoverageConnection* connection = new V8CoverageConnection(env);
192+
connection->Start();
193+
}
194+
195+
static void EndCoverageCollection(const FunctionCallbackInfo<Value>& args) {
196+
Environment* env = Environment::GetCurrent(args);
197+
CHECK(args[0]->IsFunction());
198+
Debug(env, DebugCategory::INSPECTOR_PROFILER, "Ending coverage collection\n");
199+
V8ProfilerConnection* connection = V8CoverageConnection::GetConnection(env);
200+
CHECK_NOT_NULL(connection);
201+
connection->End(args[0].As<Function>());
202+
}
203+
204+
static void Initialize(Local<Object> target,
205+
Local<Value> unused,
206+
Local<Context> context,
207+
void* priv) {
208+
Environment* env = Environment::GetCurrent(context);
209+
env->SetMethod(target, "endCoverage", EndCoverageCollection);
210+
}
211+
} // namespace profiler
212+
} // namespace node
213+
214+
NODE_MODULE_CONTEXT_AWARE_INTERNAL(profiler, node::profiler::Initialize)

‎src/node.cc

+1-3
Original file line numberDiff line numberDiff line change
@@ -245,9 +245,7 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
245245
bool rc = credentials::SafeGetenv("NODE_V8_COVERAGE", &coverage);
246246
if (rc && !coverage.empty()) {
247247
#if HAVE_INSPECTOR
248-
if (!coverage::StartCoverageCollection(env)) {
249-
return MaybeLocal<Value>();
250-
}
248+
profiler::StartCoverageCollection(env);
251249
#else
252250
fprintf(stderr, "NODE_V8_COVERAGE cannot be used without inspector");
253251
#endif // HAVE_INSPECTOR

‎src/node_binding.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@
2323
#endif
2424

2525
#if HAVE_INSPECTOR
26-
#define NODE_BUILTIN_COVERAGE_MODULES(V) V(coverage)
26+
#define NODE_BUILTIN_PROFILER_MODULES(V) V(profiler)
2727
#else
28-
#define NODE_BUILTIN_COVERAGE_MODULES(V)
28+
#define NODE_BUILTIN_PROFILER_MODULES(V)
2929
#endif
3030

3131
// A list of built-in modules. In order to do module registration
@@ -85,7 +85,7 @@
8585
NODE_BUILTIN_OPENSSL_MODULES(V) \
8686
NODE_BUILTIN_ICU_MODULES(V) \
8787
NODE_BUILTIN_REPORT_MODULES(V) \
88-
NODE_BUILTIN_COVERAGE_MODULES(V)
88+
NODE_BUILTIN_PROFILER_MODULES(V)
8989

9090
// This is used to load built-in modules. Instead of using
9191
// __attribute__((constructor)), we call the _register_<modname>

‎src/node_internals.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,8 @@ v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
296296
const char* main_script_id);
297297
v8::MaybeLocal<v8::Object> GetPerContextExports(v8::Local<v8::Context> context);
298298

299-
namespace coverage {
300-
bool StartCoverageCollection(Environment* env);
299+
namespace profiler {
300+
void StartCoverageCollection(Environment* env);
301301
}
302302

303303
#ifdef _WIN32

0 commit comments

Comments
 (0)
Please sign in to comment.