Skip to content

Commit f72ce2e

Browse files
joyeecheungtargos
authored andcommitted
src: remove fast API for InternalModuleStat
There are several motivation for removing this: 1. The implementation does not align with InternalModuleStat, most noticably it does not namespace the path or convert it to UTF-16 before using it with std::filesystem::path on Windows which could crash on non-English locale. 2. It needs the Environment - if not for decoding the string, at least for env->exec_path() to resolve the path for namespacing - and therefore needs a handle to the Context which requires a handle scope which actually makes the fast API version slower than the normal binding. For simplicity this just removes the fast API to fix the bug and improve the performance. PR-URL: #58489 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 0f1e94f commit f72ce2e

File tree

2 files changed

+1
-50
lines changed

2 files changed

+1
-50
lines changed

src/node_file.cc

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ using v8::Array;
6464
using v8::BigInt;
6565
using v8::Context;
6666
using v8::EscapableHandleScope;
67-
using v8::FastApiCallbackOptions;
6867
using v8::FunctionCallbackInfo;
6968
using v8::FunctionTemplate;
7069
using v8::HandleScope;
@@ -1073,32 +1072,6 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
10731072
args.GetReturnValue().Set(rc);
10741073
}
10751074

1076-
static int32_t FastInternalModuleStat(
1077-
Local<Value> recv,
1078-
Local<Value> input_,
1079-
// NOLINTNEXTLINE(runtime/references) This is V8 api.
1080-
FastApiCallbackOptions& options) {
1081-
TRACK_V8_FAST_API_CALL("fs.internalModuleStat");
1082-
HandleScope scope(options.isolate);
1083-
1084-
CHECK(input_->IsString());
1085-
Utf8Value input(options.isolate, input_.As<String>());
1086-
1087-
auto path = std::filesystem::path(input.ToStringView());
1088-
1089-
switch (std::filesystem::status(path).type()) {
1090-
case std::filesystem::file_type::directory:
1091-
return 1;
1092-
case std::filesystem::file_type::regular:
1093-
return 0;
1094-
default:
1095-
return -1;
1096-
}
1097-
}
1098-
1099-
v8::CFunction fast_internal_module_stat_(
1100-
v8::CFunction::Make(FastInternalModuleStat));
1101-
11021075
constexpr bool is_uv_error_except_no_entry(int result) {
11031076
return result < 0 && result != UV_ENOENT;
11041077
}
@@ -3722,11 +3695,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
37223695
SetMethod(isolate, target, "rmSync", RmSync);
37233696
SetMethod(isolate, target, "mkdir", MKDir);
37243697
SetMethod(isolate, target, "readdir", ReadDir);
3725-
SetFastMethod(isolate,
3726-
target,
3727-
"internalModuleStat",
3728-
InternalModuleStat,
3729-
&fast_internal_module_stat_);
3698+
SetMethod(isolate, target, "internalModuleStat", InternalModuleStat);
37303699
SetMethod(isolate, target, "stat", Stat);
37313700
SetMethod(isolate, target, "lstat", LStat);
37323701
SetMethod(isolate, target, "fstat", FStat);
@@ -3851,8 +3820,6 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
38513820
registry->Register(MKDir);
38523821
registry->Register(ReadDir);
38533822
registry->Register(InternalModuleStat);
3854-
registry->Register(FastInternalModuleStat);
3855-
registry->Register(fast_internal_module_stat_.GetTypeInfo());
38563823
registry->Register(Stat);
38573824
registry->Register(LStat);
38583825
registry->Register(FStat);

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

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,3 @@ const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md');
2020
const internalFsBinding = internalBinding('fs');
2121

2222
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);
38-
}

0 commit comments

Comments
 (0)