Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process: start coverage collection before bootstrap #26006

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,18 +311,5 @@ NativeModule.prototype.compile = function() {
}
};

// Coverage must be turned on early, so that we can collect
// it for Node.js' own internal libraries.
if (process.env.NODE_V8_COVERAGE) {
if (internalBinding('config').hasInspector) {
const coverage =
NativeModule.require('internal/coverage-gen/with_profiler');
// Inform the profiler to start collecting coverage
coverage.startCoverageCollection();
} else {
process._rawDebug('NODE_V8_COVERAGE cannot be used without inspector');
}
}

// This will be passed to internal/bootstrap/node.js.
return loaderExports;
46 changes: 6 additions & 40 deletions lib/internal/coverage-gen/with_profiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,9 @@
// Implements coverage collection exposed by the `NODE_V8_COVERAGE`
// environment variable which can also be used in the user land.

let coverageConnection = null;
let coverageDirectory;

function writeCoverage() {
if (!coverageConnection && coverageDirectory) {
return;
}

const { join } = require('path');
const { mkdirSync, writeFileSync } = require('fs');
const { threadId } = require('internal/worker');
Expand All @@ -28,21 +23,14 @@ function writeCoverage() {
const target = join(coverageDirectory, filename);
try {
disableAllAsyncHooks();
let msg;
coverageConnection._coverageCallback = function(_msg) {
msg = _msg;
};
coverageConnection.dispatch(JSON.stringify({
id: 3,
method: 'Profiler.takePreciseCoverage'
}));
const coverageInfo = JSON.parse(msg).result;
writeFileSync(target, JSON.stringify(coverageInfo));
internalBinding('coverage').end((msg) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to be async now? should the try/catch be moved to the callback?

const coverageInfo = JSON.parse(msg).result;
if (coverageInfo) {
writeFileSync(target, JSON.stringify(coverageInfo));
}
});
} catch (err) {
console.error(err);
} finally {
coverageConnection.disconnect();
coverageConnection = null;
}
}

Expand All @@ -52,33 +40,11 @@ function disableAllAsyncHooks() {
hooks_array.forEach((hook) => { hook.disable(); });
}

function startCoverageCollection() {
const { Connection } = internalBinding('inspector');
coverageConnection = new Connection((res) => {
if (coverageConnection._coverageCallback) {
coverageConnection._coverageCallback(res);
}
});
coverageConnection.dispatch(JSON.stringify({
id: 1,
method: 'Profiler.enable'
}));
coverageConnection.dispatch(JSON.stringify({
id: 2,
method: 'Profiler.startPreciseCoverage',
params: {
callCount: true,
detailed: true
}
}));
}

function setCoverageDirectory(dir) {
coverageDirectory = dir;
}

module.exports = {
startCoverageCollection,
writeCoverage,
setCoverageDirectory
};
9 changes: 6 additions & 3 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
V(async_wrap_ctor_template, v8::FunctionTemplate) \
V(async_wrap_object_ctor_template, v8::FunctionTemplate) \
V(buffer_prototype_object, v8::Object) \
V(coverage_connection, v8::Object) \
V(context, v8::Context) \
V(crypto_key_object_constructor, v8::Function) \
V(domain_callback, v8::Function) \
Expand Down Expand Up @@ -363,6 +364,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
V(message_port, v8::Object) \
V(message_port_constructor_template, v8::FunctionTemplate) \
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(pipe_constructor_template, v8::FunctionTemplate) \
Expand Down Expand Up @@ -447,9 +449,10 @@ struct ContextInfo {

// Listing the AsyncWrap provider types first enables us to cast directly
// from a provider type to a debug category.
#define DEBUG_CATEGORY_NAMES(V) \
NODE_ASYNC_PROVIDER_TYPES(V) \
V(INSPECTOR_SERVER)
#define DEBUG_CATEGORY_NAMES(V) \
NODE_ASYNC_PROVIDER_TYPES(V) \
V(INSPECTOR_SERVER) \
V(COVERAGE)

enum class DebugCategory {
#define V(name) name,
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 @@ -45,6 +45,7 @@
'../../src/inspector_io.cc',
'../../src/inspector_agent.h',
'../../src/inspector_io.h',
'../../src/inspector_coverage.cc',
'../../src/inspector_js_api.cc',
'../../src/inspector_socket.cc',
'../../src/inspector_socket.h',
Expand Down
168 changes: 168 additions & 0 deletions src/inspector_coverage.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
#include "base_object-inl.h"
#include "debug_utils.h"
#include "inspector_agent.h"
#include "node_internals.h"
#include "v8-inspector.h"

namespace node {
namespace coverage {

using v8::Context;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::HandleScope;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::NewStringType;
using v8::Object;
using v8::ObjectTemplate;
using v8::String;
using v8::Value;

using v8_inspector::StringBuffer;
using v8_inspector::StringView;

std::unique_ptr<StringBuffer> ToProtocolString(Isolate* isolate,
Local<Value> value) {
TwoByteValue buffer(isolate, value);
return StringBuffer::create(StringView(*buffer, buffer.length()));
}

class V8CoverageConnection : public BaseObject {
public:
class V8CoverageSessionDelegate : public inspector::InspectorSessionDelegate {
public:
explicit V8CoverageSessionDelegate(V8CoverageConnection* connection)
: connection_(connection) {}

void SendMessageToFrontend(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, seems like a huge benefit to move coverage standup and teardown into C++.

const v8_inspector::StringView& message) override {
Environment* env = connection_->env();
Local<Function> fn = connection_->env()->on_coverage_message_function();
bool ending = !fn.IsEmpty();
Debug(env,
DebugCategory::COVERAGE,
"Sending message to frontend, ending = %s\n",
ending ? "true" : "false");
if (!ending) {
return;
}
Isolate* isolate = env->isolate();

HandleScope handle_scope(isolate);
Context::Scope context_scope(env->context());
MaybeLocal<String> v8string =
String::NewFromTwoByte(isolate,
message.characters16(),
NewStringType::kNormal,
message.length());
Local<Value> args[] = {v8string.ToLocalChecked().As<Value>()};
USE(MakeCallback(isolate,
connection_->object(),
fn,
arraysize(args),
args,
async_context{0, 0}));
}

private:
V8CoverageConnection* connection_;
};

SET_MEMORY_INFO_NAME(V8CoverageConnection)
SET_SELF_SIZE(V8CoverageConnection)

void MemoryInfo(MemoryTracker* tracker) const override {
tracker->TrackFieldWithSize(
"session", sizeof(*session_), "InspectorSession");
}

explicit V8CoverageConnection(Environment* env)
: BaseObject(env, env->coverage_connection()), session_(nullptr) {
inspector::Agent* inspector = env->inspector_agent();
std::unique_ptr<inspector::InspectorSession> session = inspector->Connect(
std::make_unique<V8CoverageSessionDelegate>(this), false);
session_ = std::move(session);
MakeWeak();
}

void Start() {
Debug(this->env(),
DebugCategory::COVERAGE,
"Sending Profiler.startPreciseCoverage\n");
Isolate* isolate = this->env()->isolate();
Local<Value> enable = FIXED_ONE_BYTE_STRING(
isolate, "{\"id\": 1, \"method\": \"Profiler.enable\"}");
Local<Value> start = FIXED_ONE_BYTE_STRING(
isolate,
"{"
"\"id\": 2,"
"\"method\": \"Profiler.startPreciseCoverage\","
"\"params\": {\"callCount\": true, \"detailed\": true}"
"}");
session_->Dispatch(ToProtocolString(isolate, enable)->string());
session_->Dispatch(ToProtocolString(isolate, start)->string());
}

void End() {
Debug(this->env(),
DebugCategory::COVERAGE,
"Sending Profiler.takePreciseCoverage\n");
Isolate* isolate = this->env()->isolate();
Local<Value> end =
FIXED_ONE_BYTE_STRING(isolate,
"{"
"\"id\": 3,"
"\"method\": \"Profiler.takePreciseCoverage\""
"}");
session_->Dispatch(ToProtocolString(isolate, end)->string());
}

friend class V8CoverageSessionDelegate;

private:
std::unique_ptr<inspector::InspectorSession> session_;
};

bool StartCoverageCollection(Environment* env) {
HandleScope scope(env->isolate());

Local<ObjectTemplate> t = ObjectTemplate::New(env->isolate());
t->SetInternalFieldCount(1);
Local<Object> obj;
if (!t->NewInstance(env->context()).ToLocal(&obj)) {
return false;
}

obj->SetAlignedPointerInInternalField(0, nullptr);

CHECK(env->coverage_connection().IsEmpty());
env->set_coverage_connection(obj);
V8CoverageConnection* connection = new V8CoverageConnection(env);
connection->Start();
return true;
}

static void EndCoverageCollection(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
CHECK(args[0]->IsFunction());
Debug(env, DebugCategory::COVERAGE, "Ending coverage collection\n");
env->set_on_coverage_message_function(args[0].As<Function>());
V8CoverageConnection* connection =
Unwrap<V8CoverageConnection>(env->coverage_connection());
CHECK_NOT_NULL(connection);
connection->End();
}

static void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
env->SetMethod(target, "end", EndCoverageCollection);
}
} // namespace coverage
} // namespace node

NODE_MODULE_CONTEXT_AWARE_INTERNAL(coverage, node::coverage::Initialize)
13 changes: 13 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

#include "debug_utils.h"
#include "node_binding.h"
#include "node_buffer.h"
#include "node_constants.h"
Expand Down Expand Up @@ -230,6 +231,18 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
Isolate* isolate = env->isolate();
Local<Context> context = env->context();

std::string coverage;
bool rc = credentials::SafeGetenv("NODE_V8_COVERAGE", &coverage);
if (rc && !coverage.empty()) {
#if HAVE_INSPECTOR
if (!coverage::StartCoverageCollection(env)) {
return MaybeLocal<Value>();
}
#else
fprintf(stderr, "NODE_V8_COVERAGE cannot be used without inspector");
#endif // HAVE_INSPECTOR
}

// Add a reference to the global object
Local<Object> global = context->Global();

Expand Down
9 changes: 8 additions & 1 deletion src/node_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
#define NODE_BUILTIN_REPORT_MODULES(V)
#endif

#if HAVE_INSPECTOR
#define NODE_BUILTIN_COVERAGE_MODULES(V) V(coverage)
#else
#define NODE_BUILTIN_COVERAGE_MODULES(V)
#endif

// A list of built-in modules. In order to do module registration
// in node::Init(), need to add built-in modules in the following list.
// Then in binding::RegisterBuiltinModules(), it calls modules' registration
Expand Down Expand Up @@ -77,7 +83,8 @@
NODE_BUILTIN_STANDARD_MODULES(V) \
NODE_BUILTIN_OPENSSL_MODULES(V) \
NODE_BUILTIN_ICU_MODULES(V) \
NODE_BUILTIN_REPORT_MODULES(V)
NODE_BUILTIN_REPORT_MODULES(V) \
NODE_BUILTIN_COVERAGE_MODULES(V)

// This is used to load built-in modules. Instead of using
// __attribute__((constructor)), we call the _register_<modname>
Expand Down
4 changes: 3 additions & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,9 @@ void DefineZlibConstants(v8::Local<v8::Object> target);
v8::MaybeLocal<v8::Value> RunBootstrapping(Environment* env);
v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
const char* main_script_id);

namespace coverage {
bool StartCoverageCollection(Environment* env);
}
} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down