Repository: nodejs/node
Pull request: 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. This could be  the cause of https://github.com/nodejs/node/issues/56670 (or at least one of them since there are still many points in the code base where this is done incorrectly).
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.

Local numbers:

```
                                                                                      confidence improvement accuracy (*)   (**)  (***)
misc/startup-cli-version.js n=30 cli='deps/corepack/dist/corepack.js'                                -0.16 %       ±1.46% ±1.95% ±2.53%
misc/startup-cli-version.js n=30 cli='deps/npm/bin/npm-cli.js'                                **      1.88 %       ±1.32% ±1.76% ±2.29%
misc/startup-cli-version.js n=30 cli='deps/npm/bin/npx-cli.js'                               ***      2.37 %       ±1.04% ±1.39% ±1.81%
misc/startup-cli-version.js n=30 cli='tools/eslint/node_modules/eslint/bin/eslint.js'                -0.94 %       ±1.12% ±1.49% ±1.94%
misc/startup-core.js n=30 mode='process' script='benchmark/fixtures/require-builtins'                 0.68 %       ±1.69% ±2.25% ±2.92%
misc/startup-core.js n=30 mode='process' script='test/fixtures/semicolon'                             0.79 %       ±1.08% ±1.44% ±1.88%
misc/startup-core.js n=30 mode='process' script='test/fixtures/snapshot/typescript'                  -0.21 %       ±0.88% ±1.17% ±1.53%
misc/startup-core.js n=30 mode='worker' script='benchmark/fixtures/require-builtins'                 -0.29 %       ±1.04% ±1.39% ±1.80%
misc/startup-core.js n=30 mode='worker' script='test/fixtures/semicolon'                              2.43 %       ±2.98% ±4.01% ±5.32%
misc/startup-core.js n=30 mode='worker' script='test/fixtures/snapshot/typescript'                    0.12 %       ±0.70% ±0.93% ±1.21%
```
<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: src/node_file.cc
patch: @@ -64,7 +64,6 @@ using v8::Array;
 using v8::BigInt;
 using v8::Context;
 using v8::EscapableHandleScope;
-using v8::FastApiCallbackOptions;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::HandleScope;
@@ -1073,32 +1072,6 @@ static void InternalModuleStat(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(rc);
 }
 
-static int32_t FastInternalModuleStat(
-    Local<Value> recv,
-    Local<Value> input_,
-    // NOLINTNEXTLINE(runtime/references) This is V8 api.
-    FastApiCallbackOptions& options) {
-  TRACK_V8_FAST_API_CALL("fs.internalModuleStat");
-  HandleScope scope(options.isolate);
-
-  CHECK(input_->IsString());
-  Utf8Value input(options.isolate, input_.As<String>());
-
-  auto path = std::filesystem::path(input.ToStringView());
-
-  switch (std::filesystem::status(path).type()) {
-    case std::filesystem::file_type::directory:
-      return 1;
-    case std::filesystem::file_type::regular:
-      return 0;
-    default:
-      return -1;
-  }
-}
-
-v8::CFunction fast_internal_module_stat_(
-    v8::CFunction::Make(FastInternalModuleStat));
-
 constexpr bool is_uv_error_except_no_entry(int result) {
   return result < 0 && result != UV_ENOENT;
 }
@@ -3722,11 +3695,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
   SetMethod(isolate, target, "rmSync", RmSync);
   SetMethod(isolate, target, "mkdir", MKDir);
   SetMethod(isolate, target, "readdir", ReadDir);
-  SetFastMethod(isolate,
-                target,
-                "internalModuleStat",
-                InternalModuleStat,
-                &fast_internal_module_stat_);
+  SetMethod(isolate, target, "internalModuleStat", InternalModuleStat);
   SetMethod(isolate, target, "stat", Stat);
   SetMethod(isolate, target, "lstat", LStat);
   SetMethod(isolate, target, "fstat", FStat);
@@ -3851,8 +3820,6 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(MKDir);
   registry->Register(ReadDir);
   registry->Register(InternalModuleStat);
-  registry->Register(FastInternalModuleStat);
-  registry->Register(fast_internal_module_stat_.GetTypeInfo());
   registry->Register(Stat);
   registry->Register(LStat);
   registry->Register(FStat);

filename: test/parallel/test-permission-fs-internal-module-stat.js
patch: @@ -20,19 +20,3 @@ const blockedFile = fixtures.path('permission', 'deny', 'protected-file.md');
 const internalFsBinding = internalBinding('fs');
 
 strictEqual(internalFsBinding.internalModuleStat(blockedFile), 0);
-
-// Only javascript methods can be optimized through %OptimizeFunctionOnNextCall
-// This is why we surround the C++ method we want to optimize with a JS function.
-function testFastPaths(file) {
-  return internalFsBinding.internalModuleStat(file);
-}
-
-eval('%PrepareFunctionForOptimization(testFastPaths)');
-testFastPaths(blockedFile);
-eval('%OptimizeFunctionOnNextCall(testFastPaths)');
-strictEqual(testFastPaths(blockedFile), 0);
-
-if (common.isDebug) {
-  const { getV8FastApiCallCount } = internalBinding('debug');
-  strictEqual(getV8FastApiCallCount('fs.internalModuleStat'), 1);
-}