From 80c845bec06ab8de8ea695e11900bce0e3f556b3 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Sun, 10 Sep 2023 14:48:06 -0400 Subject: [PATCH] fs: add v8 fast api to `existsSync` --- benchmark/fs/bench-existsSync.js | 25 +++++++++++ lib/fs.js | 23 ++--------- lib/internal/fs/read/utf8.js | 25 ----------- lib/internal/fs/sync.js | 38 +++++++++++++++++ src/node_file_sync.cc | 55 +++++++++++++++++++++++++ src/node_file_sync.h | 8 ++++ test/parallel/test-bootstrap-modules.js | 2 +- 7 files changed, 130 insertions(+), 46 deletions(-) create mode 100644 benchmark/fs/bench-existsSync.js delete mode 100644 lib/internal/fs/read/utf8.js create mode 100644 lib/internal/fs/sync.js diff --git a/benchmark/fs/bench-existsSync.js b/benchmark/fs/bench-existsSync.js new file mode 100644 index 00000000000000..cd41b5a82231e8 --- /dev/null +++ b/benchmark/fs/bench-existsSync.js @@ -0,0 +1,25 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const tmpdir = require('../../test/common/tmpdir'); +tmpdir.refresh(); + +const paths = [ + __filename, + tmpdir.resolve(`.non-existing-file-${process.pid}`), +]; + +const bench = common.createBenchmark(main, { + n: [1e6], +}); + +function main({ n }) { + bench.start(); + for (let i = 0; i < n; i++) { + for (let j = 0; j < paths.length; j++) { + fs.existsSync(paths[j]); + } + } + bench.end(n); +} diff --git a/lib/fs.js b/lib/fs.js index b17cf4f10cd3c1..d583b74eec8fdc 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -141,7 +141,7 @@ const { validateObject, validateString, } = require('internal/validators'); -const { readFileSyncUtf8 } = require('internal/fs/read/utf8'); +const syncFs = require('internal/fs/sync'); let truncateWarn = true; let fs; @@ -290,23 +290,7 @@ ObjectDefineProperty(exists, kCustomPromisifiedSymbol, { * @returns {boolean} */ function existsSync(path) { - try { - path = getValidatedPath(path); - } catch { - return false; - } - const ctx = { path }; - const nPath = pathModule.toNamespacedPath(path); - binding.access(nPath, F_OK, undefined, ctx); - - // In case of an invalid symlink, `binding.access()` on win32 - // will **not** return an error and is therefore not enough. - // Double check with `binding.stat()`. - if (isWindows && ctx.errno === undefined) { - binding.stat(nPath, false, undefined, ctx); - } - - return ctx.errno === undefined; + return syncFs.exists(path); } function readFileAfterOpen(err, fd) { @@ -462,8 +446,7 @@ function readFileSync(path, options) { // TODO(@anonrig): Do not handle file descriptor ownership for now. if (!isUserFd && (options.encoding === 'utf8' || options.encoding === 'utf-8')) { - path = getValidatedPath(path); - return readFileSyncUtf8(pathModule.toNamespacedPath(path), stringToFlags(options.flag)); + return syncFs.readFileUtf8(path, options.flag); } const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666); diff --git a/lib/internal/fs/read/utf8.js b/lib/internal/fs/read/utf8.js deleted file mode 100644 index 027421601a5d68..00000000000000 --- a/lib/internal/fs/read/utf8.js +++ /dev/null @@ -1,25 +0,0 @@ -'use strict'; - -const { handleErrorFromBinding } = require('internal/fs/utils'); - -const syncBinding = internalBinding('fs_sync'); - -/** - * @param {string} path - * @param {number} flag - * @return {string} - */ -function readFileSyncUtf8(path, flag) { - const response = syncBinding.readFileUtf8(path, flag); - - if (typeof response === 'string') { - return response; - } - - const { 0: errno, 1: syscall } = response; - handleErrorFromBinding({ errno, syscall, path }); -} - -module.exports = { - readFileSyncUtf8, -}; diff --git a/lib/internal/fs/sync.js b/lib/internal/fs/sync.js new file mode 100644 index 00000000000000..bc389885414b37 --- /dev/null +++ b/lib/internal/fs/sync.js @@ -0,0 +1,38 @@ +'use strict'; + +const pathModule = require('path'); +const { handleErrorFromBinding, getValidatedPath, stringToFlags } = require('internal/fs/utils'); + +const syncBinding = internalBinding('fs_sync'); + +/** + * @param {string} path + * @param {number} flag + * @return {string} + */ +function readFileUtf8(path, flag) { + path = pathModule.toNamespacedPath(getValidatedPath(path)); + const response = syncBinding.readFileUtf8(path, stringToFlags(flag)); + + if (typeof response === 'string') { + return response; + } + + const { 0: errno, 1: syscall } = response; + handleErrorFromBinding({ errno, syscall, path }); +} + +function exists(path) { + try { + path = getValidatedPath(path); + } catch { + return false; + } + + return syncBinding.exists(pathModule.toNamespacedPath(path)); +} + +module.exports = { + readFileUtf8, + exists, +}; diff --git a/src/node_file_sync.cc b/src/node_file_sync.cc index dfd51f984dc65c..c0053d5ff29156 100644 --- a/src/node_file_sync.cc +++ b/src/node_file_sync.cc @@ -1,3 +1,4 @@ +#include "node_constants.h" #include "node_file_sync.h" #include "memory_tracker-inl.h" #include "node_buffer.h" @@ -16,16 +17,22 @@ #include +#if !defined(_MSC_VER) +#include +#endif + namespace node { namespace fs_sync { using v8::Array; +using v8::CFunction; using v8::Context; using v8::FunctionCallbackInfo; using v8::HandleScope; using v8::Int32; using v8::Integer; using v8::Isolate; +using v8::FastOneByteString; using v8::JustVoid; using v8::Local; using v8::Maybe; @@ -115,6 +122,50 @@ void BindingData::Deserialize(v8::Local context, CHECK_NOT_NULL(binding); } +bool BindingData::ExistsInternal(const std::string_view path) { + uv_fs_t req; + auto make = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); }); + FS_SYNC_TRACE_BEGIN(access); + int err = uv_fs_access(nullptr, &req, path.data(), F_OK, nullptr); + FS_SYNC_TRACE_END(access); + +#ifdef _WIN32 + // In case of an invalid symlink, `binding.access()` on win32 + // will **not** return an error and is therefore not enough. + // Double check with `stat()`. + if (err != 0) { + FS_SYNC_TRACE_BEGIN(stat); + err = uv_fs_stat(nullptr, &req, path.data, nullptr); + FS_SYNC_TRACE_END(stat); + } +#endif // _WIN32 + + return err == 0; +} + +void BindingData::Exists(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + + const int argc = args.Length(); + CHECK_GE(argc, 1); + + BufferValue path(isolate, args[0]); + CHECK_NOT_NULL(*path); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemRead, path.ToStringView()); + + args.GetReturnValue().Set(ExistsInternal(path.ToStringView())); +} + +bool BindingData::FastExists(Local receiver, + const FastOneByteString& path) { + // TODO(@anonrig): Add "THROW_IF_INSUFFICIENT_PERMISSIONS" + return ExistsInternal(std::string_view(path.data, path.length)); +} + +CFunction BindingData::fast_exists_(CFunction::Make(FastExists)); + void BindingData::ReadFileUtf8(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); auto isolate = env->isolate(); @@ -186,6 +237,7 @@ void BindingData::ReadFileUtf8(const FunctionCallbackInfo& args) { void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data, Local target) { Isolate* isolate = isolate_data->isolate(); + SetFastMethodNoSideEffect(isolate, target, "exists", Exists, &fast_exists_); SetMethodNoSideEffect(isolate, target, "readFileUtf8", ReadFileUtf8); } @@ -199,6 +251,9 @@ void BindingData::CreatePerContextProperties(Local target, void BindingData::RegisterExternalReferences( ExternalReferenceRegistry* registry) { + registry->Register(Exists); + registry->Register(FastExists); + registry->Register(fast_exists_.GetTypeInfo()); registry->Register(ReadFileUtf8); } diff --git a/src/node_file_sync.h b/src/node_file_sync.h index 364142189e98e6..89f0c059ad2d1f 100644 --- a/src/node_file_sync.h +++ b/src/node_file_sync.h @@ -31,6 +31,9 @@ class BindingData : public SnapshotableObject { SET_SELF_SIZE(BindingData) SET_MEMORY_INFO_NAME(BindingData) + static void Exists(const v8::FunctionCallbackInfo& args); + static bool FastExists(v8::Local receiver, const v8::FastOneByteString& path); + static void ReadFileUtf8(const v8::FunctionCallbackInfo& args); static void CreatePerIsolateProperties(IsolateData* isolate_data, @@ -40,6 +43,11 @@ class BindingData : public SnapshotableObject { v8::Local context, void* priv); static void RegisterExternalReferences(ExternalReferenceRegistry* registry); + + private: + static v8::CFunction fast_exists_; + + static bool ExistsInternal(const std::string_view path); }; } // namespace fs_sync diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index fe19309ff840b6..2181ea5ac7b5df 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -75,7 +75,7 @@ const expectedModules = new Set([ 'NativeModule internal/webstreams/queuingstrategies', 'NativeModule internal/blob', 'NativeModule internal/fs/utils', - 'NativeModule internal/fs/read/utf8', + 'NativeModule internal/fs/sync', 'NativeModule fs', 'Internal Binding options', 'NativeModule internal/options',