Commit f72ce2e
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
2 files changed
Lines changed: 1 addition & 50 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -64,7 +64,6 @@ using v8::Array; | |||
| 64 | 64 | using v8::BigInt; | |
| 65 | 65 | using v8::Context; | |
| 66 | 66 | using v8::EscapableHandleScope; | |
| 67 | - using v8::FastApiCallbackOptions; | ||
| 68 | 67 | using v8::FunctionCallbackInfo; | |
| 69 | 68 | using v8::FunctionTemplate; | |
| 70 | 69 | using v8::HandleScope; | |
@@ -1073,32 +1072,6 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) { | |||
| 1073 | 1072 | args.GetReturnValue().Set(rc); | |
| 1074 | 1073 | } | |
| 1075 | 1074 | ||
| 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 | - | ||
| 1102 | 1075 | constexpr bool is_uv_error_except_no_entry(int result) { | |
| 1103 | 1076 | return result < 0 && result != UV_ENOENT; | |
| 1104 | 1077 | } | |
@@ -3722,11 +3695,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, | |||
| 3722 | 3695 | SetMethod(isolate, target, "rmSync", RmSync); | |
| 3723 | 3696 | SetMethod(isolate, target, "mkdir", MKDir); | |
| 3724 | 3697 | 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); | ||
| 3730 | 3699 | SetMethod(isolate, target, "stat", Stat); | |
| 3731 | 3700 | SetMethod(isolate, target, "lstat", LStat); | |
| 3732 | 3701 | SetMethod(isolate, target, "fstat", FStat); | |
@@ -3851,8 +3820,6 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { | |||
| 3851 | 3820 | registry->Register(MKDir); | |
| 3852 | 3821 | registry->Register(ReadDir); | |
| 3853 | 3822 | registry->Register(InternalModuleStat); | |
| 3854 | - registry->Register(FastInternalModuleStat); | ||
| 3855 | - registry->Register(fast_internal_module_stat_.GetTypeInfo()); | ||
| 3856 | 3823 | registry->Register(Stat); | |
| 3857 | 3824 | registry->Register(LStat); | |
| 3858 | 3825 | registry->Register(FStat); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -20,19 +20,3 @@ const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md'); | |||
| 20 | 20 | const internalFsBinding = internalBinding('fs'); | |
| 21 | 21 | ||
| 22 | 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); | ||
| 38 | - } | ||
0 commit comments