Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up require(), phase 1 and 2 #1801

Merged
merged 2 commits into from
May 27, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 19 additions & 24 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const runInThisContext = require('vm').runInThisContext;
const assert = require('assert').ok;
const fs = require('fs');
const path = require('path');
const internalModuleReadFile = process.binding('fs').internalModuleReadFile;
const internalModuleStat = process.binding('fs').internalModuleStat;


// If obj.hasOwnProperty has been overridden, then calling
Expand Down Expand Up @@ -56,13 +58,6 @@ const debug = Module._debug;
// -> a.<ext>
// -> a/index.<ext>

function statPath(path) {
try {
return fs.statSync(path);
} catch (ex) {}
return false;
}

// check if the directory is a package.json dir
const packageMainCache = {};

Expand All @@ -71,10 +66,10 @@ function readPackage(requestPath) {
return packageMainCache[requestPath];
}

try {
var jsonPath = path.resolve(requestPath, 'package.json');
var json = fs.readFileSync(jsonPath, 'utf8');
} catch (e) {
var jsonPath = path.resolve(requestPath, 'package.json');
var json = internalModuleReadFile(jsonPath);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two other places where fs.readFileSync could be replaced but that would alter the stack trace in case of error.

I hope and assume no one writes code that depends on the exact stack trace but I decided to let it be for now, just in case.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that should be a concern, especially not when there's potential perf gains. Nobody should depend on the exact stack trace.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really disagree but the other two call sites don't seem to be bottlenecks like the package.json load step is.


if (json === undefined) {
return false;
}

Expand All @@ -94,7 +89,7 @@ function tryPackage(requestPath, exts) {
if (!pkg) return false;

var filename = path.resolve(requestPath, pkg);
return tryFile(filename, null) || tryExtensions(filename, exts) ||
return tryFile(filename) || tryExtensions(filename, exts) ||
tryExtensions(path.resolve(filename, 'index'), exts);
}

Expand All @@ -104,18 +99,19 @@ function tryPackage(requestPath, exts) {
Module._realpathCache = {};

// check if the file exists and is not a directory
function tryFile(requestPath, stats) {
stats = stats || statPath(requestPath);
if (stats && !stats.isDirectory()) {
return fs.realpathSync(requestPath, Module._realpathCache);
}
return false;
function tryFile(requestPath) {
const rc = internalModuleStat(requestPath);
return rc === 0 && toRealPath(requestPath);
}

function toRealPath(requestPath) {
return fs.realpathSync(requestPath, Module._realpathCache);
}

// given a path check a the file exists with any of the set extensions
function tryExtensions(p, exts) {
for (var i = 0, EL = exts.length; i < EL; i++) {
var filename = tryFile(p + exts[i], null);
var filename = tryFile(p + exts[i]);

if (filename) {
return filename;
Expand Down Expand Up @@ -150,11 +146,10 @@ Module._findPath = function(request, paths) {
var filename;

if (!trailingSlash) {
var stats = statPath(basePath);
// try to join the request to the path
filename = tryFile(basePath, stats);

if (!filename && stats && stats.isDirectory()) {
const rc = internalModuleStat(basePath);
if (rc === 0) { // File.
filename = toRealPath(basePath);
} else if (rc === 1) { // Directory.
filename = tryPackage(basePath, exts);
}

Expand Down
68 changes: 68 additions & 0 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
# include <io.h>
#endif

#include <vector>

namespace node {

using v8::Array;
Expand Down Expand Up @@ -433,6 +435,70 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) {
return handle_scope.Escape(stats);
}

// 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.
static void InternalModuleReadFile(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK(args[0]->IsString());
node::Utf8Value path(env->isolate(), args[0]);

FILE* const stream = fopen(*path, "rb");
if (stream == nullptr) {
return;
}

std::vector<char> chars;
while (!ferror(stream)) {
const size_t kBlockSize = 32 << 10;
const size_t start = chars.size();
chars.resize(start + kBlockSize);
const size_t numchars = fread(&chars[start], 1, kBlockSize, stream);
if (numchars < kBlockSize) {
chars.resize(start + numchars);
}
if (numchars == 0) {
break;
}
}

CHECK_EQ(false, ferror(stream));
CHECK_EQ(0, fclose(stream));

size_t start = 0;
if (chars.size() >= 3 && 0 == memcmp(&chars[0], "\xEF\xBB\xBF", 3)) {
start = 3; // Skip UTF-8 BOM.
}

Local<String> chars_string =
String::NewFromUtf8(env->isolate(),
&chars[start],
String::kNormalString,
chars.size() - start);
args.GetReturnValue().Set(chars_string);
}

// Used to speed up module loading. Returns 0 if the path refers to
// a file, 1 when it's a directory or < 0 on error (usually -ENOENT.)
// The speedup comes from not creating thousands of Stat and Error objects.
static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK(args[0]->IsString());
node::Utf8Value path(env->isolate(), args[0]);

uv_fs_t req;
int rc = uv_fs_stat(env->event_loop(), &req, *path, nullptr);
if (rc == 0) {
const uv_stat_t* const s = static_cast<const uv_stat_t*>(req.ptr);
rc = !!(s->st_mode & S_IFDIR);
}
uv_fs_req_cleanup(&req);

args.GetReturnValue().Set(rc);
}

static void Stat(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand Down Expand Up @@ -1141,6 +1207,8 @@ void InitFs(Handle<Object> target,
env->SetMethod(target, "rmdir", RMDir);
env->SetMethod(target, "mkdir", MKDir);
env->SetMethod(target, "readdir", ReadDir);
env->SetMethod(target, "internalModuleReadFile", InternalModuleReadFile);
env->SetMethod(target, "internalModuleStat", InternalModuleStat);
env->SetMethod(target, "stat", Stat);
env->SetMethod(target, "lstat", LStat);
env->SetMethod(target, "fstat", FStat);
Expand Down