Skip to content

Commit 889b5ca

Browse files
joyeecheungRenegade334
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: nodejs#58489 Backport-PR-URL: https://github.com/nodejs/node/pull/XXXXX Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
1 parent 0367f4b commit 889b5ca

File tree

2 files changed

+1
-58
lines changed

2 files changed

+1
-58
lines changed

src/node_file.cc

Lines changed: 1 addition & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include "aliased_buffer-inl.h"
2424
#include "memory_tracker-inl.h"
2525
#include "node_buffer.h"
26-
#include "node_debug.h"
2726
#include "node_errors.h"
2827
#include "node_external_reference.h"
2928
#include "node_file-inl.h"
@@ -60,7 +59,6 @@ using v8::Array;
6059
using v8::BigInt;
6160
using v8::Context;
6261
using v8::EscapableHandleScope;
63-
using v8::FastApiCallbackOptions;
6462
using v8::FunctionCallbackInfo;
6563
using v8::FunctionTemplate;
6664
using v8::HandleScope;
@@ -1066,39 +1064,6 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
10661064
args.GetReturnValue().Set(rc);
10671065
}
10681066

1069-
static int32_t FastInternalModuleStat(
1070-
Local<Value> recv,
1071-
Local<Value> input_,
1072-
// NOLINTNEXTLINE(runtime/references) This is V8 api.
1073-
FastApiCallbackOptions& options) {
1074-
// This needs a HandleScope which needs an isolate.
1075-
Isolate* isolate = Isolate::TryGetCurrent();
1076-
if (!isolate) {
1077-
options.fallback = true;
1078-
return -1;
1079-
}
1080-
1081-
TRACK_V8_FAST_API_CALL("fs.internalModuleStat");
1082-
HandleScope scope(isolate);
1083-
1084-
CHECK(input_->IsString());
1085-
Utf8Value input(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-
11021067
constexpr bool is_uv_error_except_no_entry(int result) {
11031068
return result < 0 && result != UV_ENOENT;
11041069
}
@@ -3828,11 +3793,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
38283793
SetMethod(isolate, target, "rmdir", RMDir);
38293794
SetMethod(isolate, target, "mkdir", MKDir);
38303795
SetMethod(isolate, target, "readdir", ReadDir);
3831-
SetFastMethod(isolate,
3832-
target,
3833-
"internalModuleStat",
3834-
InternalModuleStat,
3835-
&fast_internal_module_stat_);
3796+
SetMethod(isolate, target, "internalModuleStat", InternalModuleStat);
38363797
SetMethod(isolate, target, "stat", Stat);
38373798
SetMethod(isolate, target, "lstat", LStat);
38383799
SetMethod(isolate, target, "fstat", FStat);
@@ -3957,8 +3918,6 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
39573918
registry->Register(MKDir);
39583919
registry->Register(ReadDir);
39593920
registry->Register(InternalModuleStat);
3960-
registry->Register(FastInternalModuleStat);
3961-
registry->Register(fast_internal_module_stat_.GetTypeInfo());
39623921
registry->Register(Stat);
39633922
registry->Register(LStat);
39643923
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)