Skip to content

Commit

Permalink
src: port coverage serialization to C++
Browse files Browse the repository at this point in the history
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: #26874
Reviewed-By: Ben Coe <bencoe@gmail.com>
  • Loading branch information
joyeecheung committed Apr 6, 2019
1 parent baa54a5 commit 864860e
Show file tree
Hide file tree
Showing 20 changed files with 367 additions and 244 deletions.
12 changes: 1 addition & 11 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,20 +110,10 @@ function setupWarningHandler() {
// Setup User-facing NODE_V8_COVERAGE environment variable that writes
// ScriptCoverage to a specified file.
function setupCoverageHooks(dir) {
const originalReallyExit = process.reallyExit;
const cwd = require('internal/process/execution').tryGetCwd();
const { resolve } = require('path');
const coverageDirectory = resolve(cwd, dir);
const {
writeCoverage,
setCoverageDirectory
} = require('internal/profiler');
setCoverageDirectory(coverageDirectory);
process.on('exit', writeCoverage);
process.reallyExit = (code) => {
writeCoverage();
originalReallyExit(code);
};
internalBinding('profiler').setCoverageDirectory(coverageDirectory);
return coverageDirectory;
}

Expand Down
4 changes: 0 additions & 4 deletions lib/internal/process/per_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,6 @@ function wrapProcessMethods(binding) {

function kill(pid, sig) {
var err;
if (process.env.NODE_V8_COVERAGE) {
const { writeCoverage } = require('internal/profiler');
writeCoverage();
}

// eslint-disable-next-line eqeqeq
if (pid != (pid | 0)) {
Expand Down
45 changes: 0 additions & 45 deletions lib/internal/profiler.js

This file was deleted.

1 change: 0 additions & 1 deletion node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@
'lib/internal/process/worker_thread_only.js',
'lib/internal/process/report.js',
'lib/internal/process/task_queues.js',
'lib/internal/profiler.js',
'lib/internal/querystring.js',
'lib/internal/readline.js',
'lib/internal/repl.js',
Expand Down
20 changes: 20 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,26 @@ inline const std::vector<std::string>& Environment::exec_argv() {
return exec_argv_;
}

#if HAVE_INSPECTOR
inline void Environment::set_coverage_directory(const char* dir) {
coverage_directory_ = std::string(dir);
}

inline void Environment::set_coverage_connection(
std::unique_ptr<profiler::V8CoverageConnection> connection) {
CHECK_NULL(coverage_connection_);
std::swap(coverage_connection_, connection);
}

inline profiler::V8CoverageConnection* Environment::coverage_connection() {
return coverage_connection_.get();
}

inline const std::string& Environment::coverage_directory() const {
return coverage_directory_;
}
#endif // HAVE_INSPECTOR

inline std::shared_ptr<HostPort> Environment::inspector_host_port() {
return inspector_host_port_;
}
Expand Down
1 change: 0 additions & 1 deletion src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,6 @@ Local<Value> Environment::GetNow() {
return Number::New(isolate(), static_cast<double>(now));
}


void Environment::set_debug_categories(const std::string& cats, bool enabled) {
std::string debug_categories = cats;
while (!debug_categories.empty()) {
Expand Down
23 changes: 21 additions & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "aliased_buffer.h"
#if HAVE_INSPECTOR
#include "inspector_agent.h"
#include "inspector_profiler.h"
#endif
#include "handle_wrap.h"
#include "node.h"
Expand Down Expand Up @@ -67,6 +68,12 @@ namespace tracing {
class AgentWriterHandle;
}

#if HAVE_INSPECTOR
namespace profiler {
class V8CoverageConnection;
} // namespace profiler
#endif // HAVE_INSPECTOR

namespace worker {
class Worker;
}
Expand Down Expand Up @@ -366,7 +373,6 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
V(async_hooks_init_function, v8::Function) \
V(async_hooks_promise_resolve_function, v8::Function) \
V(buffer_prototype_object, v8::Object) \
V(coverage_connection, v8::Object) \
V(crypto_key_object_constructor, v8::Function) \
V(domain_callback, v8::Function) \
V(domexception_function, v8::Function) \
Expand All @@ -390,7 +396,6 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
V(inspector_console_extension_installer, v8::Function) \
V(message_port, v8::Object) \
V(native_module_require, v8::Function) \
V(on_coverage_message_function, v8::Function) \
V(performance_entry_callback, v8::Function) \
V(performance_entry_template, v8::Function) \
V(process_object, v8::Object) \
Expand Down Expand Up @@ -1116,6 +1121,15 @@ class Environment : public MemoryRetainer {

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

#if HAVE_INSPECTOR
void set_coverage_connection(
std::unique_ptr<profiler::V8CoverageConnection> connection);
profiler::V8CoverageConnection* coverage_connection();

inline void set_coverage_directory(const char* directory);
inline const std::string& coverage_directory() const;
#endif // HAVE_INSPECTOR

private:
inline void CreateImmediate(native_immediate_callback cb,
void* data,
Expand Down Expand Up @@ -1146,6 +1160,11 @@ class Environment : public MemoryRetainer {
size_t async_callback_scope_depth_ = 0;
std::vector<double> destroy_async_id_list_;

#if HAVE_INSPECTOR
std::unique_ptr<profiler::V8CoverageConnection> coverage_connection_;
std::string coverage_directory_;
#endif // HAVE_INSPECTOR

std::shared_ptr<EnvironmentOptions> options_;
// options_ contains debug options parsed from CLI arguments,
// while inspector_host_port_ stores the actual inspector host
Expand Down
1 change: 1 addition & 0 deletions src/inspector/node_inspector.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
'../../src/inspector_io.cc',
'../../src/inspector_agent.h',
'../../src/inspector_io.h',
'../../src/inspector_profiler.h',
'../../src/inspector_profiler.cc',
'../../src/inspector_js_api.cc',
'../../src/inspector_socket.cc',
Expand Down
Loading

0 comments on commit 864860e

Please sign in to comment.