Skip to content

Commit 864860e

Browse files
joyeecheungjoyeecheung
committed
src: port coverage serialization to C++
This patch moves the serialization of coverage profiles into C++. With this we no longer need to patch `process.reallyExit` and hook into the exit events, but instead hook into relevant places in C++ which are safe from user manipulation. This also makes the code easier to reuse for other types of profiles. PR-URL: nodejs#26874 Reviewed-By: Ben Coe <bencoe@gmail.com>
1 parent baa54a5 commit 864860e

20 files changed

Lines changed: 367 additions & 244 deletions

‎lib/internal/bootstrap/pre_execution.js‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -110,20 +110,10 @@ function setupWarningHandler() {
110110
// Setup User-facing NODE_V8_COVERAGE environment variable that writes
111111
// ScriptCoverage to a specified file.
112112
function setupCoverageHooks(dir) {
113-
const originalReallyExit = process.reallyExit;
114113
const cwd = require('internal/process/execution').tryGetCwd();
115114
const { resolve } = require('path');
116115
const coverageDirectory = resolve(cwd, dir);
117-
const {
118-
writeCoverage,
119-
setCoverageDirectory
120-
} = require('internal/profiler');
121-
setCoverageDirectory(coverageDirectory);
122-
process.on('exit', writeCoverage);
123-
process.reallyExit = (code) => {
124-
writeCoverage();
125-
originalReallyExit(code);
126-
};
116+
internalBinding('profiler').setCoverageDirectory(coverageDirectory);
127117
return coverageDirectory;
128118
}
129119

‎lib/internal/process/per_thread.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,6 @@ function wrapProcessMethods(binding) {
157157

158158
function kill(pid, sig) {
159159
var err;
160-
if (process.env.NODE_V8_COVERAGE) {
161-
const { writeCoverage } = require('internal/profiler');
162-
writeCoverage();
163-
}
164160

165161
// eslint-disable-next-line eqeqeq
166162
if (pid != (pid | 0)) {

‎lib/internal/profiler.js‎

Lines changed: 0 additions & 45 deletions
This file was deleted.

‎node.gyp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@
169169
'lib/internal/process/worker_thread_only.js',
170170
'lib/internal/process/report.js',
171171
'lib/internal/process/task_queues.js',
172-
'lib/internal/profiler.js',
173172
'lib/internal/querystring.js',
174173
'lib/internal/readline.js',
175174
'lib/internal/repl.js',

‎src/env-inl.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,26 @@ inline const std::vector<std::string>& Environment::exec_argv() {
638638
return exec_argv_;
639639
}
640640

641+
#if HAVE_INSPECTOR
642+
inline void Environment::set_coverage_directory(const char* dir) {
643+
coverage_directory_ = std::string(dir);
644+
}
645+
646+
inline void Environment::set_coverage_connection(
647+
std::unique_ptr<profiler::V8CoverageConnection> connection) {
648+
CHECK_NULL(coverage_connection_);
649+
std::swap(coverage_connection_, connection);
650+
}
651+
652+
inline profiler::V8CoverageConnection* Environment::coverage_connection() {
653+
return coverage_connection_.get();
654+
}
655+
656+
inline const std::string& Environment::coverage_directory() const {
657+
return coverage_directory_;
658+
}
659+
#endif // HAVE_INSPECTOR
660+
641661
inline std::shared_ptr<HostPort> Environment::inspector_host_port() {
642662
return inspector_host_port_;
643663
}

‎src/env.cc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,6 @@ Local<Value> Environment::GetNow() {
715715
return Number::New(isolate(), static_cast<double>(now));
716716
}
717717

718-
719718
void Environment::set_debug_categories(const std::string& cats, bool enabled) {
720719
std::string debug_categories = cats;
721720
while (!debug_categories.empty()) {

‎src/env.h‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "aliased_buffer.h"
2828
#if HAVE_INSPECTOR
2929
#include "inspector_agent.h"
30+
#include "inspector_profiler.h"
3031
#endif
3132
#include "handle_wrap.h"
3233
#include "node.h"
@@ -67,6 +68,12 @@ namespace tracing {
6768
class AgentWriterHandle;
6869
}
6970

71+
#if HAVE_INSPECTOR
72+
namespace profiler {
73+
class V8CoverageConnection;
74+
} // namespace profiler
75+
#endif // HAVE_INSPECTOR
76+
7077
namespace worker {
7178
class Worker;
7279
}
@@ -366,7 +373,6 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
366373
V(async_hooks_init_function, v8::Function) \
367374
V(async_hooks_promise_resolve_function, v8::Function) \
368375
V(buffer_prototype_object, v8::Object) \
369-
V(coverage_connection, v8::Object) \
370376
V(crypto_key_object_constructor, v8::Function) \
371377
V(domain_callback, v8::Function) \
372378
V(domexception_function, v8::Function) \
@@ -390,7 +396,6 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
390396
V(inspector_console_extension_installer, v8::Function) \
391397
V(message_port, v8::Object) \
392398
V(native_module_require, v8::Function) \
393-
V(on_coverage_message_function, v8::Function) \
394399
V(performance_entry_callback, v8::Function) \
395400
V(performance_entry_template, v8::Function) \
396401
V(process_object, v8::Object) \
@@ -1116,6 +1121,15 @@ class Environment : public MemoryRetainer {
11161121

11171122
inline AsyncRequest* thread_stopper() { return &thread_stopper_; }
11181123

1124+
#if HAVE_INSPECTOR
1125+
void set_coverage_connection(
1126+
std::unique_ptr<profiler::V8CoverageConnection> connection);
1127+
profiler::V8CoverageConnection* coverage_connection();
1128+
1129+
inline void set_coverage_directory(const char* directory);
1130+
inline const std::string& coverage_directory() const;
1131+
#endif // HAVE_INSPECTOR
1132+
11191133
private:
11201134
inline void CreateImmediate(native_immediate_callback cb,
11211135
void* data,
@@ -1146,6 +1160,11 @@ class Environment : public MemoryRetainer {
11461160
size_t async_callback_scope_depth_ = 0;
11471161
std::vector<double> destroy_async_id_list_;
11481162

1163+
#if HAVE_INSPECTOR
1164+
std::unique_ptr<profiler::V8CoverageConnection> coverage_connection_;
1165+
std::string coverage_directory_;
1166+
#endif // HAVE_INSPECTOR
1167+
11491168
std::shared_ptr<EnvironmentOptions> options_;
11501169
// options_ contains debug options parsed from CLI arguments,
11511170
// while inspector_host_port_ stores the actual inspector host

‎src/inspector/node_inspector.gypi‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
'../../src/inspector_io.cc',
4545
'../../src/inspector_agent.h',
4646
'../../src/inspector_io.h',
47+
'../../src/inspector_profiler.h',
4748
'../../src/inspector_profiler.cc',
4849
'../../src/inspector_js_api.cc',
4950
'../../src/inspector_socket.cc',

0 commit comments

Comments
 (0)