GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — src: fix internalModuleStat v8 fast path · nodejs/node@d712aa4

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

lib
- fs.js
- internal
  - fs
    - promises.js
  - modules
    - cjs
      - loader.js
    - esm
      - resolve.js
    - package_json_reader.js
src
- node_external_reference.h
- node_file.cc
test/parallel
- test-permission-fs-internal-module-stat.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎lib/fs.js‎`

Copy file name to clipboardExpand all lines: lib/fs.js

+2-2Lines changed: 2 additions & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -1428,7 +1428,7 @@ function handleDirents({ result, currentPath, context }) {`
`1428`	`1428`	`const dirent = getDirent(currentPath, names[i], types[i]);`
`1429`	`1429`	`ArrayPrototypePush(context.readdirResults, dirent);`
`1430`	`1430`
`1431`		`- if (dirent.isDirectory() \|\| binding.internalModuleStat(binding, fullPath) === 1) {`
	`1431`	`+ if (dirent.isDirectory() \|\| binding.internalModuleStat(fullPath) === 1) {`
`1432`	`1432`	`ArrayPrototypePush(context.pathsQueue, fullPath);`
`1433`	`1433`	`}`
`1434`	`1434`	`}`
`@@ -1438,7 +1438,7 @@ function handleFilePaths({ result, currentPath, context }) {`
`1438`	`1438`	`for (let i = 0; i < result.length; i++) {`
`1439`	`1439`	`const resultPath = pathModule.join(currentPath, result[i]);`
`1440`	`1440`	`const relativeResultPath = pathModule.relative(context.basePath, resultPath);`
`1441`		`- const stat = binding.internalModuleStat(binding, resultPath);`
	`1441`	`+ const stat = binding.internalModuleStat(resultPath);`
`1442`	`1442`	`ArrayPrototypePush(context.readdirResults, relativeResultPath);`
`1443`	`1443`
`1444`	`1444`	`if (stat === 1) {`

Collapse file

`‎lib/internal/fs/promises.js‎`

Copy file name to clipboardExpand all lines: lib/internal/fs/promises.js

+1-1Lines changed: 1 addition & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -910,7 +910,7 @@ async function readdirRecursive(originalPath, options) {`
`910`	`910`	`const { 0: path, 1: readdir } = ArrayPrototypePop(queue);`
`911`	`911`	`for (const ent of readdir) {`
`912`	`912`	`const direntPath = pathModule.join(path, ent);`
`913`		`- const stat = binding.internalModuleStat(binding, direntPath);`
	`913`	`+ const stat = binding.internalModuleStat(direntPath);`
`914`	`914`	`ArrayPrototypePush(`
`915`	`915`	`result,`
`916`	`916`	`pathModule.relative(originalPath, direntPath),`

Collapse file

`‎lib/internal/modules/cjs/loader.js‎`

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js

+2-2Lines changed: 2 additions & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -255,9 +255,9 @@ function stat(filename) {`
`255`	`255`	`const result = statCache.get(filename);`
`256`	`256`	`if (result !== undefined) { return result; }`
`257`	`257`	`}`
`258`		`- const result = internalFsBinding.internalModuleStat(internalFsBinding, filename);`
	`258`	`+ const result = internalFsBinding.internalModuleStat(filename);`
`259`	`259`	`if (statCache !== null && result >= 0) {`
`260`		- // Only set cache when `internalModuleStat(internalFsBinding, filename)` succeeds.
	`260`	+ // Only set cache when `internalModuleStat(filename)` succeeds.
`261`	`261`	`statCache.set(filename, result);`
`262`	`262`	`}`
`263`	`263`	`return result;`

Collapse file

`‎lib/internal/modules/esm/resolve.js‎`

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/resolve.js

-1Lines changed: 0 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -248,7 +248,6 @@ function finalizeResolution(resolved, base, preserveSymlinks) {`
`248`	`248`	`}`
`249`	`249`
`250`	`250`	`const stats = internalFsBinding.internalModuleStat(`
`251`		`- internalFsBinding,`
`252`	`251`	`StringPrototypeEndsWith(internalFsBinding, path, '/') ? StringPrototypeSlice(path, -1) : path,`
`253`	`252`	`);`
`254`	`253`

Collapse file

`‎lib/internal/modules/package_json_reader.js‎`

Copy file name to clipboardExpand all lines: lib/internal/modules/package_json_reader.js

-1Lines changed: 0 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -235,7 +235,6 @@ function getPackageJSONURL(specifier, base) {`
`235`	`235`	`let lastPath;`
`236`	`236`	`do {`
`237`	`237`	`const stat = internalFsBinding.internalModuleStat(`
`238`		`- internalFsBinding,`
`239`	`238`	`StringPrototypeSlice(packageJSONPath, 0, packageJSONPath.length - 13),`
`240`	`239`	`);`
`241`	`240`	`// Check for !stat.isDirectory()`

Collapse file

`‎src/node_external_reference.h‎`

Copy file name to clipboardExpand all lines: src/node_external_reference.h

+2-3Lines changed: 2 additions & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -32,9 +32,8 @@ using CFunctionCallback = void (*)(v8::Local<v8::Value> unused,`
`32`	`32`	`using CFunctionCallbackReturnDouble =`
`33`	`33`	`double (*)(v8::Local<v8::Object> unused, v8::Local<v8::Object> receiver);`
`34`	`34`	`using CFunctionCallbackReturnInt32 =`
`35`		`- int32_t (*)(v8::Local<v8::Object> unused,`
`36`		`- v8::Local<v8::Object> receiver,`
`37`		`- const v8::FastOneByteString& input,`
	`35`	`+ int32_t (*)(v8::Local<v8::Value> receiver,`
	`36`	`+ v8::Local<v8::Value> input,`
`38`	`37`	`// NOLINTNEXTLINE(runtime/references) This is V8 api.`
`39`	`38`	`v8::FastApiCallbackOptions& options);`
`40`	`39`	`using CFunctionCallbackValueReturnDouble =`

Collapse file

`‎src/node_file.cc‎`

Copy file name to clipboardExpand all lines: src/node_file.cc

+12-11Lines changed: 12 additions & 11 deletions

Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@`
`23`	`23`	`#include "aliased_buffer-inl.h"`
`24`	`24`	`#include "memory_tracker-inl.h"`
`25`	`25`	`#include "node_buffer.h"`
	`26`	`+#include "node_debug.h"`
`26`	`27`	`#include "node_errors.h"`
`27`	`28`	`#include "node_external_reference.h"`
`28`	`29`	`#include "node_file-inl.h"`
`@@ -63,8 +64,6 @@ using v8::BigInt;`
`63`	`64`	`using v8::Context;`
`64`	`65`	`using v8::EscapableHandleScope;`
`65`	`66`	`using v8::FastApiCallbackOptions;`
`66`		`-using v8::FastOneByteString;`
`67`		`-using v8::Function;`
`68`	`67`	`using v8::FunctionCallbackInfo;`
`69`	`68`	`using v8::FunctionTemplate;`
`70`	`69`	`using v8::HandleScope;`
`@@ -1056,9 +1055,9 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& args) {`
`1056`	`1055`	`static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {`
`1057`	`1056`	`Environment* env = Environment::GetCurrent(args);`
`1058`	`1057`
`1059`		`- CHECK_GE(args.Length(), 2);`
`1060`		`- CHECK(args[1]->IsString());`
`1061`		`- BufferValue path(env->isolate(), args[1]);`
	`1058`	`+ CHECK_EQ(args.Length(), 1);`
	`1059`	`+ CHECK(args[0]->IsString());`
	`1060`	`+ BufferValue path(env->isolate(), args[0]);`
`1062`	`1061`	`CHECK_NOT_NULL(*path);`
`1063`	`1062`	`ToNamespacedPath(env, &path);`
`1064`	`1063`
`@@ -1074,15 +1073,17 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {`
`1074`	`1073`	`}`
`1075`	`1074`
`1076`	`1075`	`static int32_t FastInternalModuleStat(`
`1077`		`- Local<Object> unused,`
`1078`		`- Local<Object> recv,`
`1079`		`- const FastOneByteString& input,`
	`1076`	`+ Local<Value> recv,`
	`1077`	`+ Local<Value> input_,`
`1080`	`1078`	`// NOLINTNEXTLINE(runtime/references) This is V8 api.`
`1081`	`1079`	`FastApiCallbackOptions& options) {`
`1082`		`- Environment* env = Environment::GetCurrent(options.isolate);`
`1083`		`- HandleScope scope(env->isolate());`
	`1080`	`+ TRACK_V8_FAST_API_CALL("fs.internalModuleStat");`
	`1081`	`+ HandleScope scope(options.isolate);`
	`1082`	`+`
	`1083`	`+ CHECK(input_->IsString());`
	`1084`	`+ Utf8Value input(options.isolate, input_.As<String>());`
`1084`	`1085`
`1085`		`- auto path = std::filesystem::path(input.data, input.data + input.length);`
	`1086`	`+ auto path = std::filesystem::path(input.ToStringView());`
`1086`	`1087`
`1087`	`1088`	`switch (std::filesystem::status(path).type()) {`
`1088`	`1089`	`case std::filesystem::file_type::directory:`

Collapse file

`‎test/parallel/test-permission-fs-internal-module-stat.js‎`

Copy file name to clipboardExpand all lines: test/parallel/test-permission-fs-internal-module-stat.js

+18-6Lines changed: 18 additions & 6 deletions

Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,9 @@`
`1`		`-// Flags: --expose-internals --permission --allow-fs-read=test/common* --allow-fs-read=tools* --allow-fs-read=test/parallel* --allow-child-process`
	`1`	`+// Flags: --expose-internals --permission --allow-fs-read=test/common* --allow-fs-read=tools* --allow-fs-read=test/parallel* --allow-child-process --allow-natives-syntax`
`2`	`2`	`'use strict';`
`3`	`3`
`4`	`4`	`const common = require('../common');`
`5`	`5`	`const { isMainThread } = require('worker_threads');`
	`6`	`+const { strictEqual } = require('assert');`
`6`	`7`
`7`	`8`	`if (!isMainThread) {`
`8`	`9`	`common.skip('This test only works on a main thread');`
`@@ -18,9 +19,20 @@ const fixtures = require('../common/fixtures');`
`18`	`19`	`const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md');`
`19`	`20`	`const internalFsBinding = internalBinding('fs');`
`20`	`21`
`21`		`-// Run this inside a for loop to trigger the fast API`
`22`		`-for (let i = 0; i < 10_000; i++) {`
`23`		`- // internalModuleStat does not use permission model.`
`24`		`- // doesNotThrow`
`25`		`- internalFsBinding.internalModuleStat(internalFsBinding, blockedFile);`
	`22`	`+strictEqual(internalFsBinding.internalModuleStat(blockedFile), 0);`
	`23`	`+`
	`24`	`+// Only javascript methods can be optimized through %OptimizeFunctionOnNextCall`
	`25`	`+// This is why we surround the C++ method we want to optimize with a JS function.`
	`26`	`+function testFastPaths(file) {`
	`27`	`+ return internalFsBinding.internalModuleStat(file);`
	`28`	`+}`
	`29`	`+`
	`30`	`+eval('%PrepareFunctionForOptimization(testFastPaths)');`
	`31`	`+testFastPaths(blockedFile);`
	`32`	`+eval('%OptimizeFunctionOnNextCall(testFastPaths)');`
	`33`	`+strictEqual(testFastPaths(blockedFile), 0);`
	`34`	`+`
	`35`	`+if (common.isDebug) {`
	`36`	`+ const { getV8FastApiCallCount } = internalBinding('debug');`
	`37`	`+ strictEqual(getV8FastApiCallCount('fs.internalModuleStat'), 1);`
`26`	`38`	`}`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.