Skip to content

Commit d712aa4

Browse files
anonrigRafaelGSS
authored andcommitted
src: fix internalModuleStat v8 fast path
PR-URL: #58054 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
1 parent ca86c93 commit d712aa4

8 files changed

Lines changed: 37 additions & 27 deletions

File tree

‎lib/fs.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,7 +1428,7 @@ function handleDirents({ result, currentPath, context }) {
14281428
constdirent=getDirent(currentPath,names[i],types[i]);
14291429
ArrayPrototypePush(context.readdirResults,dirent);
14301430

1431-
if(dirent.isDirectory()||binding.internalModuleStat(binding,fullPath)===1){
1431+
if(dirent.isDirectory()||binding.internalModuleStat(fullPath)===1){
14321432
ArrayPrototypePush(context.pathsQueue,fullPath);
14331433
}
14341434
}
@@ -1438,7 +1438,7 @@ function handleFilePaths({ result, currentPath, context }) {
14381438
for(leti=0;i<result.length;i++){
14391439
constresultPath=pathModule.join(currentPath,result[i]);
14401440
constrelativeResultPath=pathModule.relative(context.basePath,resultPath);
1441-
conststat=binding.internalModuleStat(binding,resultPath);
1441+
conststat=binding.internalModuleStat(resultPath);
14421442
ArrayPrototypePush(context.readdirResults,relativeResultPath);
14431443

14441444
if(stat===1){

‎lib/internal/fs/promises.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ async function readdirRecursive(originalPath, options) {
910910
const{0: path,1: readdir}=ArrayPrototypePop(queue);
911911
for(constentofreaddir){
912912
constdirentPath=pathModule.join(path,ent);
913-
conststat=binding.internalModuleStat(binding,direntPath);
913+
conststat=binding.internalModuleStat(direntPath);
914914
ArrayPrototypePush(
915915
result,
916916
pathModule.relative(originalPath,direntPath),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,9 @@ function stat(filename) {
255255
constresult=statCache.get(filename);
256256
if(result!==undefined){returnresult;}
257257
}
258-
constresult=internalFsBinding.internalModuleStat(internalFsBinding,filename);
258+
constresult=internalFsBinding.internalModuleStat(filename);
259259
if(statCache!==null&&result>=0){
260-
// Only set cache when `internalModuleStat(internalFsBinding, filename)` succeeds.
260+
// Only set cache when `internalModuleStat(filename)` succeeds.
261261
statCache.set(filename,result);
262262
}
263263
returnresult;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,6 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
248248
}
249249

250250
conststats=internalFsBinding.internalModuleStat(
251-
internalFsBinding,
252251
StringPrototypeEndsWith(internalFsBinding,path,'/') ? StringPrototypeSlice(path,-1) : path,
253252
);
254253

‎lib/internal/modules/package_json_reader.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,6 @@ function getPackageJSONURL(specifier, base) {
235235
letlastPath;
236236
do{
237237
conststat=internalFsBinding.internalModuleStat(
238-
internalFsBinding,
239238
StringPrototypeSlice(packageJSONPath,0,packageJSONPath.length-13),
240239
);
241240
// Check for !stat.isDirectory()

‎src/node_external_reference.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ using CFunctionCallback = void (*)(v8::Local<v8::Value> unused,
3232
using CFunctionCallbackReturnDouble =
3333
double (*)(v8::Local<v8::Object> unused, v8::Local<v8::Object> receiver);
3434
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,
3837
// NOLINTNEXTLINE(runtime/references) This is V8 api.
3938
v8::FastApiCallbackOptions& options);
4039
using CFunctionCallbackValueReturnDouble =

‎src/node_file.cc‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include"aliased_buffer-inl.h"
2424
#include"memory_tracker-inl.h"
2525
#include"node_buffer.h"
26+
#include"node_debug.h"
2627
#include"node_errors.h"
2728
#include"node_external_reference.h"
2829
#include"node_file-inl.h"
@@ -63,8 +64,6 @@ using v8::BigInt;
6364
using v8::Context;
6465
using v8::EscapableHandleScope;
6566
using v8::FastApiCallbackOptions;
66-
using v8::FastOneByteString;
67-
using v8::Function;
6867
using v8::FunctionCallbackInfo;
6968
using v8::FunctionTemplate;
7069
using v8::HandleScope;
@@ -1056,9 +1055,9 @@ static void ExistsSync(const FunctionCallbackInfo<Value>& args) {
10561055
staticvoidInternalModuleStat(const FunctionCallbackInfo<Value>& args) {
10571056
Environment* env = Environment::GetCurrent(args);
10581057

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]);
10621061
CHECK_NOT_NULL(*path);
10631062
ToNamespacedPath(env, &path);
10641063

@@ -1074,15 +1073,17 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
10741073
}
10751074

10761075
staticint32_tFastInternalModuleStat(
1077-
Local<Object> unused,
1078-
Local<Object> recv,
1079-
const FastOneByteString& input,
1076+
Local<Value> recv,
1077+
Local<Value> input_,
10801078
// NOLINTNEXTLINE(runtime/references) This is V8 api.
10811079
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>());
10841085

1085-
auto path = std::filesystem::path(input.data, input.data + input.length);
1086+
auto path = std::filesystem::path(input.ToStringView());
10861087

10871088
switch (std::filesystem::status(path).type()) {
10881089
case std::filesystem::file_type::directory:

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff 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
22
'use strict';
33

44
constcommon=require('../common');
55
const{ isMainThread }=require('worker_threads');
6+
const{ strictEqual }=require('assert');
67

78
if(!isMainThread){
89
common.skip('This test only works on a main thread');
@@ -18,9 +19,20 @@ const fixtures = require('../common/fixtures');
1819
constblockedFile=fixtures.path('permission','deny','protected-file.md');
1920
constinternalFsBinding=internalBinding('fs');
2021

21-
// Run this inside a for loop to trigger the fast API
22-
for(leti=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+
functiontestFastPaths(file){
27+
returninternalFsBinding.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);
2638
}

0 commit comments

Comments
 (0)