Skip to content

Commit 0e58916

Browse files
lemirerichardlau
authored andcommitted
src: stop including simdjson.h from env.h
env.h includes inspector_profiler.h, which included simdjson.h for the types of two virtual methods and a parser member. As a result, about 155 files in src/ parsed the amalgamated simdjson header, which is over 10 MB. Keep simdjson out of inspector_profiler.h: the profile connections now say which field of `result` holds the profile, and WriteProfile() takes the profile as raw JSON. The parser becomes a local variable, so its buffers are no longer kept alive for the lifetime of the Environment. Also drop the unused include from node_task_runner.h, and let the config file reader rewind its document instead of parsing the whole file a second time with a second parser. Now only the eight files that use simdjson include it. On an Apple M-series machine, a clean rebuild of the src/ objects takes about 15% less CPU time (376 s to 320 s) and 19% less wall time at -j14. Signed-off-by: Daniel Lemire <daniel@lemire.me> PR-URL: #66620 Refs: simdjson/simdjson#2913 Refs: #66495 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <richard.lau@ibm.com>
1 parent f44aec4 commit 0e58916

5 files changed

Lines changed: 45 additions & 70 deletions

File tree

‎src/inspector_profiler.cc‎

Lines changed: 32 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313

1414
#include <cinttypes>
1515
#include <limits>
16+
#include <optional>
1617
#include <sstream>
18+
#include "simdjson.h"
1719
#include "simdutf.h"
1820

1921
namespace node {
@@ -109,6 +111,28 @@ bool StringViewToUTF8(const v8_inspector::StringView& source,
109111
return *utf8_length == result_len;
110112
}
111113

114+
// Return the raw JSON of the profile object in `result`: either the
115+
// `field` member of `result` or, if `field` is nullptr, `result` itself.
116+
static std::optional<std::string_view> GetProfile(
117+
simdjson::ondemand::object* result, const char* field, const char* type) {
118+
simdjson::ondemand::object profile_object;
119+
if (field == nullptr) {
120+
profile_object = *result;
121+
} else if ((*result)[field].get_object().get(profile_object)) {
122+
fprintf(
123+
stderr, "'%s' from %s profile result is not an Object\n", field, type);
124+
return std::nullopt;
125+
}
126+
std::string_view profile_raw;
127+
if (profile_object.raw_json().get(profile_raw)) {
128+
fprintf(stderr,
129+
"Cannot get raw string of the 'profile' field from %s profile\n",
130+
type);
131+
return std::nullopt;
132+
}
133+
return profile_raw;
134+
}
135+
112136
void V8ProfilerConnection::V8ProfilerSessionDelegate::SendMessageToFrontend(
113137
const v8_inspector::StringView& message) {
114138
Environment* env = connection_->env();
@@ -134,9 +158,10 @@ void V8ProfilerConnection::V8ProfilerSessionDelegate::SendMessageToFrontend(
134158
return;
135159
}
136160

161+
simdjson::ondemand::parser json_parser;
137162
simdjson::ondemand::document parsed;
138163
simdjson::ondemand::object response;
139-
if (connection_->json_parser_
164+
if (json_parser
140165
.iterate(
141166
message_utf8.data(), message_utf8_length, message_utf8.size())
142167
.get(parsed) ||
@@ -184,7 +209,10 @@ void V8ProfilerConnection::V8ProfilerSessionDelegate::SendMessageToFrontend(
184209
return;
185210
}
186211

187-
connection_->WriteProfile(&result);
212+
auto profile = GetProfile(&result, connection_->profile_field(), type);
213+
if (profile.has_value()) {
214+
connection_->WriteProfile(*profile);
215+
}
188216
connection_->RemoveProfileId(id);
189217
}
190218

@@ -214,32 +242,7 @@ std::string V8CoverageConnection::GetFilename() const {
214242
env()->thread_id());
215243
}
216244

217-
std::optional<std::string_view> V8ProfilerConnection::GetProfile(
218-
simdjson::ondemand::object* result) {
219-
simdjson::ondemand::object profile_object;
220-
if ((*result)["profile"].get_object().get(profile_object)) {
221-
fprintf(
222-
stderr, "'profile' from %s profile result is not an Object\n", type());
223-
return std::nullopt;
224-
}
225-
std::string_view profile_raw;
226-
if (profile_object.raw_json().get(profile_raw)) {
227-
fprintf(stderr,
228-
"Cannot get raw string of the 'profile' field from %s profile\n",
229-
type());
230-
return std::nullopt;
231-
}
232-
return profile_raw;
233-
}
234-
235-
void V8ProfilerConnection::WriteProfile(simdjson::ondemand::object* result) {
236-
// Generate the profile output from the subclass.
237-
auto profile_opt = GetProfile(result);
238-
if (!profile_opt.has_value()) {
239-
return;
240-
}
241-
std::string_view profile = profile_opt.value();
242-
245+
void V8ProfilerConnection::WriteProfile(std::string_view profile) {
243246
// Create the directory if necessary.
244247
std::string directory = GetDirectory();
245248
DCHECK(!directory.empty());
@@ -254,7 +257,7 @@ void V8ProfilerConnection::WriteProfile(simdjson::ondemand::object* result) {
254257
WriteResult(env_, path.c_str(), profile);
255258
}
256259

257-
void V8CoverageConnection::WriteProfile(simdjson::ondemand::object* result) {
260+
void V8CoverageConnection::WriteProfile(std::string_view profile) {
258261
Isolate* isolate = env_->isolate();
259262
HandleScope handle_scope(isolate);
260263

@@ -271,13 +274,6 @@ void V8CoverageConnection::WriteProfile(simdjson::ondemand::object* result) {
271274
Local<Context> context = env_->context();
272275
Context::Scope context_scope(context);
273276

274-
// Generate the profile output from the subclass.
275-
auto profile_opt = GetProfile(result);
276-
if (!profile_opt.has_value()) {
277-
return;
278-
}
279-
std::string_view profile = profile_opt.value();
280-
281277
// append source-map cache information to coverage object:
282278
Local<Value> source_map_cache_v;
283279
{
@@ -349,18 +345,6 @@ void V8CoverageConnection::WriteProfile(simdjson::ondemand::object* result) {
349345
}
350346
}
351347

352-
std::optional<std::string_view> V8CoverageConnection::GetProfile(
353-
simdjson::ondemand::object* result) {
354-
std::string_view profile_raw;
355-
if (result->raw_json().get(profile_raw)) {
356-
fprintf(stderr,
357-
"Cannot get raw string of the 'profile' field from %s profile\n",
358-
type());
359-
return std::nullopt;
360-
}
361-
return profile_raw;
362-
}
363-
364348
std::string V8CoverageConnection::GetDirectory() const {
365349
return env()->coverage_directory();
366350
}

‎src/inspector_profiler.h‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@
77
#error("This header can only be used when inspector is enabled")
88
#endif
99

10-
#include <optional>
10+
#include <string_view>
1111
#include <unordered_set>
1212
#include "inspector_agent.h"
13-
#include "simdjson.h"
1413

1514
namespace node {
1615
// Forward declaration to break recursive dependency chain with src/env.h.
@@ -59,11 +58,11 @@ class V8ProfilerConnection {
5958
virtual std::string GetDirectory() const = 0;
6059
// Return the filename the profile should be written as.
6160
virtual std::string GetFilename() const = 0;
62-
// Return the profile object parsed from `message.result`,
63-
// which will be then written as a JSON.
64-
virtual std::optional<std::string_view> GetProfile(
65-
simdjson::ondemand::object* result);
66-
virtual void WriteProfile(simdjson::ondemand::object* result);
61+
// Return the field of `message.result` that holds the profile object,
62+
// or nullptr if `message.result` is itself the profile object.
63+
virtual const char* profile_field() const { return "profile"; }
64+
// Write the profile, given as raw JSON.
65+
virtual void WriteProfile(std::string_view profile);
6766

6867
bool HasProfileId(uint64_t id) const { return profile_ids_.contains(id); }
6968

@@ -76,7 +75,6 @@ class V8ProfilerConnection {
7675
std::unordered_set<uint64_t> profile_ids_;
7776

7877
protected:
79-
simdjson::ondemand::parser json_parser_;
8078
Environment* env_ = nullptr;
8179
};
8280

@@ -92,9 +90,8 @@ class V8CoverageConnection : public V8ProfilerConnection {
9290

9391
std::string GetDirectory() const override;
9492
std::string GetFilename() const override;
95-
std::optional<std::string_view> GetProfile(
96-
simdjson::ondemand::object* result) override;
97-
void WriteProfile(simdjson::ondemand::object* result) override;
93+
const char* profile_field() const override { return nullptr; }
94+
void WriteProfile(std::string_view profile) override;
9895
void WriteSourceMapCache();
9996
void TakeCoverage();
10097
void StopCoverage();

‎src/node_config_file.cc‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -573,19 +573,13 @@ ParseResult ConfigReader::ParseConfig(const std::string_view& config_path) {
573573
return ParseResult::InvalidContent;
574574
}
575575

576-
simdjson::ondemand::parser config_parser;
577-
simdjson::ondemand::document config_document;
578-
if (config_parser.iterate(file_content).get(config_document)) {
579-
FPrintF(stderr, "Can't parse %s\n", config_path.data());
580-
return ParseResult::InvalidContent;
581-
}
582-
583-
simdjson::ondemand::object config_object;
584-
if (config_document.get_object().get(config_object)) {
576+
// Walk the document again, this time to parse the configuration.
577+
document.rewind();
578+
if (document.get_object().get(main_object)) {
585579
return ParseResult::InvalidContent;
586580
}
587581

588-
return ParseConfigObject(&config_object, config_path, true);
582+
return ParseConfigObject(&main_object, config_path, true);
589583
}
590584

591585
std::string ConfigReader::GetNodeOptions() {

‎src/node_task_runner.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "node_task_runner.h"
2+
#include "simdjson.h"
23
#include "util-inl.h"
34

45
#include <regex> // NOLINT(build/c++11)

‎src/node_task_runner.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
55

66
#include "node_internals.h"
7-
#include "simdjson.h"
87
#include "spawn_sync.h"
98
#include "uv.h"
109

0 commit comments

Comments
 (0)