From 0fdd88a374e23e1dd4a05d93afd5eb0c3b080fd5 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 9 Nov 2017 13:26:47 +0100 Subject: [PATCH] module: speed up package.json parsing more Move the logic from the previous commit to C++ land in order to avoid creating a new string when we know we won't parse it anyway. PR-URL: https://github.com/nodejs/node/pull/15767 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/module.js | 2 +- src/node_file.cc | 8 +++++--- src/string_search.h | 11 ++++++++++- test/parallel/test-module-binding.js | 5 +++++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/lib/module.js b/lib/module.js index b9de9c3edfb782..cc8d5097bb83b2 100644 --- a/lib/module.js +++ b/lib/module.js @@ -120,7 +120,7 @@ function readPackage(requestPath) { return false; } - if (!/"main"/.test(json)) + if (json === '') return packageMainCache[requestPath] = undefined; try { diff --git a/src/node_file.cc b/src/node_file.cc index 76ab7f9f337829..216b60485663b6 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -25,6 +25,7 @@ #include "req-wrap-inl.h" #include "string_bytes.h" +#include "string_search.h" #include #include @@ -466,8 +467,9 @@ void FillStatsArray(double* fields, const uv_stat_t* s) { } // Used to speed up module loading. Returns the contents of the file as -// a string or undefined when the file cannot be opened. The speedup -// comes from not creating Error objects on failure. +// a string or undefined when the file cannot be opened. Returns an empty +// string when the file does not contain the substring '"main"' because that +// is the property we care about. static void InternalModuleReadFile(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); uv_loop_t* loop = env->event_loop(); @@ -516,7 +518,7 @@ static void InternalModuleReadFile(const FunctionCallbackInfo& args) { } const size_t size = offset - start; - if (size == 0) { + if (size == 0 || size == SearchString(&chars[start], size, "\"main\"")) { args.GetReturnValue().SetEmptyString(); } else { Local chars_string = diff --git a/src/string_search.h b/src/string_search.h index 6e76a5b8f413ea..51f72062a99466 100644 --- a/src/string_search.h +++ b/src/string_search.h @@ -638,6 +638,7 @@ size_t SearchString(const Char* haystack, size_t needle_length, size_t start_index, bool is_forward) { + if (haystack_length < needle_length) return haystack_length; // To do a reverse search (lastIndexOf instead of indexOf) without redundant // code, create two vectors that are reversed views into the input strings. // For example, v_needle[0] would return the *last* character of the needle. @@ -646,7 +647,6 @@ size_t SearchString(const Char* haystack, needle, needle_length, is_forward); Vector v_haystack = Vector( haystack, haystack_length, is_forward); - CHECK(haystack_length >= needle_length); size_t diff = haystack_length - needle_length; size_t relative_start_index; if (is_forward) { @@ -664,6 +664,15 @@ size_t SearchString(const Char* haystack, } return is_forward ? pos : (haystack_length - needle_length - pos); } + +template +size_t SearchString(const char* haystack, size_t haystack_length, + const char (&needle)[N]) { + return SearchString( + reinterpret_cast(haystack), haystack_length, + reinterpret_cast(needle), N - 1, 0, true); +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/test/parallel/test-module-binding.js b/test/parallel/test-module-binding.js index ba11c3e47ec646..f5c2a794b110c7 100644 --- a/test/parallel/test-module-binding.js +++ b/test/parallel/test-module-binding.js @@ -2,8 +2,13 @@ require('../common'); const fixtures = require('../common/fixtures'); const { internalModuleReadFile } = process.binding('fs'); +const { readFileSync } = require('fs'); const { strictEqual } = require('assert'); strictEqual(internalModuleReadFile('nosuchfile'), undefined); strictEqual(internalModuleReadFile(fixtures.path('empty.txt')), ''); strictEqual(internalModuleReadFile(fixtures.path('empty-with-bom.txt')), ''); +{ + const filename = fixtures.path('require-bin/package.json'); + strictEqual(internalModuleReadFile(filename), readFileSync(filename, 'utf8')); +}