Skip to content

Commit

Permalink
module: speed up package.json parsing more
Browse files Browse the repository at this point in the history
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 <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
bnoordhuis committed Nov 15, 2017
1 parent fdbb6dd commit 0fdd88a
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 5 deletions.
2 changes: 1 addition & 1 deletion lib/module.js
Expand Up @@ -120,7 +120,7 @@ function readPackage(requestPath) {
return false;
}

if (!/"main"/.test(json))
if (json === '')
return packageMainCache[requestPath] = undefined;

try {
Expand Down
8 changes: 5 additions & 3 deletions src/node_file.cc
Expand Up @@ -25,6 +25,7 @@

#include "req-wrap-inl.h"
#include "string_bytes.h"
#include "string_search.h"

#include <fcntl.h>
#include <sys/types.h>
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 =
Expand Down
11 changes: 10 additions & 1 deletion src/string_search.h
Expand Up @@ -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.
Expand All @@ -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) {
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-module-binding.js
Expand Up @@ -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'));
}

0 comments on commit 0fdd88a

Please sign in to comment.