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: #15767 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Nov 15, 2017 · 0fdd88a · 0fdd88a
1 parent fdbb6dd
commit 0fdd88a
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 5 deletions.
diff --git 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
@@ -25,6 +25,7 @@
 
 #include "req-wrap-inl.h"
 #include "string_bytes.h"
+#include "string_search.h"
 
 #include <fcntl.h>
 #include <sys/types.h>
@@ -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<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   uv_loop_t* loop = env->event_loop();
@@ -516,7 +518,7 @@ static void InternalModuleReadFile(const FunctionCallbackInfo<Value>& args) {
   }
 
   const size_t size = offset - start;
-  if (size == 0) {
+  if (size == 0 || size == SearchString(&chars[start], size, "\"main\"")) {
     args.GetReturnValue().SetEmptyString();
   } else {
     Local<String> chars_string =

diff --git 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<const Char> v_haystack = Vector<const Char>(
       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 N>
+size_t SearchString(const char* haystack, size_t haystack_length,
+                    const char (&needle)[N]) {
+  return SearchString(
+      reinterpret_cast<const uint8_t*>(haystack), haystack_length,
+      reinterpret_cast<const uint8_t*>(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
@@ -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'));
+}