From 230e98b54a2e5345155018c1ff2e8530e9011913 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 8 Feb 2019 19:56:08 +0800 Subject: [PATCH] process: start coverage collection before bootstrap This patch moves the dispatch of `Profiler.takePreciseCoverage` to a point before the bootstrap scripts are run to ensure that we can collect coverage data for all the scripts run after the inspector agent is ready. Before this patch `lib/internal/bootstrap/primordials.js` was not covered by `make coverage`, after this patch it is. PR-URL: https://github.com/nodejs/node/pull/26006 Reviewed-By: Anna Henningsen Reviewed-By: Ben Coe Reviewed-By: James M Snell --- lib/internal/bootstrap/loaders.js | 13 -- lib/internal/coverage-gen/with_profiler.js | 46 +----- src/env.h | 9 +- src/inspector/node_inspector.gypi | 1 + src/inspector_coverage.cc | 168 +++++++++++++++++++++ src/node.cc | 13 ++ src/node_binding.cc | 9 +- src/node_internals.h | 4 +- 8 files changed, 205 insertions(+), 58 deletions(-) create mode 100644 src/inspector_coverage.cc diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index d21fddbf5239a1..28f7f5277b9ee0 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -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; diff --git a/lib/internal/coverage-gen/with_profiler.js b/lib/internal/coverage-gen/with_profiler.js index 6b17073939b658..573d2c3712b281 100644 --- a/lib/internal/coverage-gen/with_profiler.js +++ b/lib/internal/coverage-gen/with_profiler.js @@ -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'); @@ -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) => { + const coverageInfo = JSON.parse(msg).result; + if (coverageInfo) { + writeFileSync(target, JSON.stringify(coverageInfo)); + } + }); } catch (err) { console.error(err); - } finally { - coverageConnection.disconnect(); - coverageConnection = null; } } @@ -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 }; diff --git a/src/env.h b/src/env.h index 749ad22b67267f..626b43121b44ee 100644 --- a/src/env.h +++ b/src/env.h @@ -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) \ @@ -364,6 +365,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2; V(message_event_object_template, v8::ObjectTemplate) \ 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) \ @@ -448,9 +450,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, diff --git a/src/inspector/node_inspector.gypi b/src/inspector/node_inspector.gypi index 21f98cd9002cab..9483a0712e22bd 100644 --- a/src/inspector/node_inspector.gypi +++ b/src/inspector/node_inspector.gypi @@ -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', diff --git a/src/inspector_coverage.cc b/src/inspector_coverage.cc new file mode 100644 index 00000000000000..8cbec586fba718 --- /dev/null +++ b/src/inspector_coverage.cc @@ -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 ToProtocolString(Isolate* isolate, + Local 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( + const v8_inspector::StringView& message) override { + Environment* env = connection_->env(); + Local 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 v8string = + String::NewFromTwoByte(isolate, + message.characters16(), + NewStringType::kNormal, + message.length()); + Local args[] = {v8string.ToLocalChecked().As()}; + 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 session = inspector->Connect( + std::make_unique(this), false); + session_ = std::move(session); + MakeWeak(); + } + + void Start() { + Debug(this->env(), + DebugCategory::COVERAGE, + "Sending Profiler.startPreciseCoverage\n"); + Isolate* isolate = this->env()->isolate(); + Local enable = FIXED_ONE_BYTE_STRING( + isolate, "{\"id\": 1, \"method\": \"Profiler.enable\"}"); + Local 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 end = + FIXED_ONE_BYTE_STRING(isolate, + "{" + "\"id\": 3," + "\"method\": \"Profiler.takePreciseCoverage\"" + "}"); + session_->Dispatch(ToProtocolString(isolate, end)->string()); + } + + friend class V8CoverageSessionDelegate; + + private: + std::unique_ptr session_; +}; + +bool StartCoverageCollection(Environment* env) { + HandleScope scope(env->isolate()); + + Local t = ObjectTemplate::New(env->isolate()); + t->SetInternalFieldCount(1); + Local 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& 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()); + V8CoverageConnection* connection = + Unwrap(env->coverage_connection()); + CHECK_NOT_NULL(connection); + connection->End(); +} + +static void Initialize(Local target, + Local unused, + Local 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) diff --git a/src/node.cc b/src/node.cc index f257495a1351e2..82282e202bec48 100644 --- a/src/node.cc +++ b/src/node.cc @@ -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" @@ -230,6 +231,18 @@ MaybeLocal RunBootstrapping(Environment* env) { Isolate* isolate = env->isolate(); Local 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(); + } +#else + fprintf(stderr, "NODE_V8_COVERAGE cannot be used without inspector"); +#endif // HAVE_INSPECTOR + } + // Add a reference to the global object Local global = context->Global(); diff --git a/src/node_binding.cc b/src/node_binding.cc index 08d55567d4904a..c9ff7be46f80fc 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -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 @@ -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_ diff --git a/src/node_internals.h b/src/node_internals.h index 6cbad8959415aa..01ebd8b40af3a2 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -269,7 +269,9 @@ void DefineZlibConstants(v8::Local target); v8::MaybeLocal RunBootstrapping(Environment* env); v8::MaybeLocal StartExecution(Environment* env, const char* main_script_id); - +namespace coverage { +bool StartCoverageCollection(Environment* env); +} } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS