fs: improve error perf of sync *times

PR-URL: #49864
Refs: nodejs/performance#106
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Author: CanadaHonk

benchmark/fs/bench-timesSync.js

@@ -0,0 +1,61 @@
+'use strict';
+
+constcommon=require('../common');
+constfs=require('fs');
+consttmpdir=require('../../test/common/tmpdir');
+tmpdir.refresh();
+
+constbench=common.createBenchmark(main,{
+type: ['existing','non-existing'],
+func: ['utimes','futimes','lutimes'],
+n: [1e3],
+});
+
+functionmain({ n, type, func }){
+constuseFds=func==='futimes';
+constfsFunc=fs[func+'Sync'];
+
+switch(type){
+case'existing': {
+constfiles=[];
+
+// Populate tmpdir with mock files
+for(leti=0;i<n;i++){
+constpath=tmpdir.resolve(`timessync-bench-file-${i}`);
+fs.writeFileSync(path,'bench');
+files.push(useFds ? fs.openSync(path,'r+') : path);
+}
+
+bench.start();
+for(leti=0;i<n;i++){
+fsFunc(files[i],i,i);
+}
+bench.end(n);
+
+if(useFds)files.forEach((x)=>{
+try{
+fs.closeSync(x);
+}catch{
+// do nothing
+}
+});
+
+break;
+}
+case'non-existing': {
+bench.start();
+for(leti=0;i<n;i++){
+try{
+fsFunc(useFds ? (1<<30) : tmpdir.resolve(`.non-existing-file-${Date.now()}`),i,i);
+}catch{
+// do nothing
+}
+}
+bench.end(n);
+
+break;
+}
+default:
+newError('Invalid type');
+}
+}

lib/fs.js

@@ -2123,11 +2123,11 @@ function utimes(path, atime, mtime, callback) {
  */
 functionutimesSync(path,atime,mtime){
 path=getValidatedPath(path);
-constctx={ path };
-binding.utimes(pathModule.toNamespacedPath(path),
-toUnixTimestamp(atime),toUnixTimestamp(mtime),
-undefined,ctx);
-handleErrorFromBinding(ctx);
+binding.utimes(
+pathModule.toNamespacedPath(path),
+toUnixTimestamp(atime),
+toUnixTimestamp(mtime),
+);
 }
 
 /**
@@ -2160,12 +2160,11 @@ function futimes(fd, atime, mtime, callback) {
  * @returns {void}
  */
 functionfutimesSync(fd,atime,mtime){
-fd=getValidatedFd(fd);
-atime=toUnixTimestamp(atime,'atime');
-mtime=toUnixTimestamp(mtime,'mtime');
-constctx={};
-binding.futimes(fd,atime,mtime,undefined,ctx);
-handleErrorFromBinding(ctx);
+binding.futimes(
+getValidatedFd(fd),
+toUnixTimestamp(atime,'atime'),
+toUnixTimestamp(mtime,'mtime'),
+);
 }
 
 /**
@@ -2199,12 +2198,11 @@ function lutimes(path, atime, mtime, callback) {
  */
 functionlutimesSync(path,atime,mtime){
 path=getValidatedPath(path);
-constctx={ path };
-binding.lutimes(pathModule.toNamespacedPath(path),
-toUnixTimestamp(atime),
-toUnixTimestamp(mtime),
-undefined,ctx);
-handleErrorFromBinding(ctx);
+binding.lutimes(
+pathModule.toNamespacedPath(path),
+toUnixTimestamp(atime),
+toUnixTimestamp(mtime),
+);
 }
 
 functionwriteAll(fd,isUserFd,buffer,offset,length,signal,flush,callback){

src/node_file.cc

@@ -2721,18 +2721,17 @@ static void UTimes(const FunctionCallbackInfo<Value>& args) {
 CHECK(args[2]->IsNumber());
 constdouble mtime = args[2].As<Number>()->Value();
 
-FSReqBase* req_wrap_async = GetReqWrap(args, 3);
-if (req_wrap_async != nullptr) { // utimes(path, atime, mtime, req)
+if (argc > 3) { // utimes(path, atime, mtime, req)
+ FSReqBase* req_wrap_async = GetReqWrap(args, 3);
 FS_ASYNC_TRACE_BEGIN1(
 UV_FS_UTIME, req_wrap_async, "path", TRACE_STR_COPY(*path))
 AsyncCall(env, req_wrap_async, args, "utime", UTF8, AfterNoArgs,
  uv_fs_utime, *path, atime, mtime);
- } else { // utimes(path, atime, mtime, undefined, ctx)
-CHECK_EQ(argc, 5);
- FSReqWrapSync req_wrap_sync;
+ } else { // utimes(path, atime, mtime)
+ FSReqWrapSync req_wrap_sync("utime", *path);
 FS_SYNC_TRACE_BEGIN(utimes);
-SyncCall(env, args[4], &req_wrap_sync, "utime",
- uv_fs_utime, *path, atime, mtime);
+SyncCallAndThrowOnError(
+env, &req_wrap_sync, uv_fs_utime, *path, atime, mtime);
 FS_SYNC_TRACE_END(utimes);
  }
 }
@@ -2752,17 +2751,16 @@ static void FUTimes(const FunctionCallbackInfo<Value>& args) {
 CHECK(args[2]->IsNumber());
 constdouble mtime = args[2].As<Number>()->Value();
 
-FSReqBase* req_wrap_async = GetReqWrap(args, 3);
-if (req_wrap_async != nullptr) { // futimes(fd, atime, mtime, req)
+if (argc > 3) { // futimes(fd, atime, mtime, req)
+ FSReqBase* req_wrap_async = GetReqWrap(args, 3);
 FS_ASYNC_TRACE_BEGIN0(UV_FS_FUTIME, req_wrap_async)
 AsyncCall(env, req_wrap_async, args, "futime", UTF8, AfterNoArgs,
  uv_fs_futime, fd, atime, mtime);
- } else { // futimes(fd, atime, mtime, undefined, ctx)
-CHECK_EQ(argc, 5);
- FSReqWrapSync req_wrap_sync;
+ } else { // futimes(fd, atime, mtime)
+ FSReqWrapSync req_wrap_sync("futime");
 FS_SYNC_TRACE_BEGIN(futimes);
-SyncCall(env, args[4], &req_wrap_sync, "futime",
- uv_fs_futime, fd, atime, mtime);
+SyncCallAndThrowOnError(
+env, &req_wrap_sync, uv_fs_futime, fd, atime, mtime);
 FS_SYNC_TRACE_END(futimes);
  }
 }
@@ -2784,18 +2782,17 @@ static void LUTimes(const FunctionCallbackInfo<Value>& args) {
 CHECK(args[2]->IsNumber());
 constdouble mtime = args[2].As<Number>()->Value();
 
-FSReqBase* req_wrap_async = GetReqWrap(args, 3);
-if (req_wrap_async != nullptr) { // lutimes(path, atime, mtime, req)
+if (argc > 3) { // lutimes(path, atime, mtime, req)
+ FSReqBase* req_wrap_async = GetReqWrap(args, 3);
 FS_ASYNC_TRACE_BEGIN1(
 UV_FS_LUTIME, req_wrap_async, "path", TRACE_STR_COPY(*path))
 AsyncCall(env, req_wrap_async, args, "lutime", UTF8, AfterNoArgs,
  uv_fs_lutime, *path, atime, mtime);
- } else { // lutimes(path, atime, mtime, undefined, ctx)
-CHECK_EQ(argc, 5);
- FSReqWrapSync req_wrap_sync;
+ } else { // lutimes(path, atime, mtime)
+ FSReqWrapSync req_wrap_sync("lutime", *path);
 FS_SYNC_TRACE_BEGIN(lutimes);
-SyncCall(env, args[4], &req_wrap_sync, "lutime",
- uv_fs_lutime, *path, atime, mtime);
+SyncCallAndThrowOnError(
+env, &req_wrap_sync, uv_fs_lutime, *path, atime, mtime);
 FS_SYNC_TRACE_END(lutimes);
  }
 }

typings/internalBinding/fs.d.ts

@@ -107,7 +107,7 @@ declare namespace InternalFSBinding {
 functionftruncate(fd: number,len: number,usePromises: typeofkUsePromises): Promise<void>;
 
 functionfutimes(fd: number,atime: number,mtime: number,req: FSReqCallback): void;
-functionfutimes(fd: number,atime: number,mtime: number,req: undefined,ctx: FSSyncContext): void;
+functionfutimes(fd: number,atime: number,mtime: number): void;
 functionfutimes(fd: number,atime: number,mtime: number,usePromises: typeofkUsePromises): Promise<void>;
 
 functioninternalModuleReadJSON(path: string): []|[string,boolean];
@@ -132,7 +132,7 @@ declare namespace InternalFSBinding {
 functionlstat(path: StringOrBuffer,useBigint: false,usePromises: typeofkUsePromises): Promise<Float64Array>;
 
 functionlutimes(path: string,atime: number,mtime: number,req: FSReqCallback): void;
-functionlutimes(path: string,atime: number,mtime: number,req: undefined,ctx: FSSyncContext): void;
+functionlutimes(path: string,atime: number,mtime: number): void;
 functionlutimes(path: string,atime: number,mtime: number,usePromises: typeofkUsePromises): Promise<void>;
 
 functionmkdtemp(prefix: string,encoding: unknown,req: FSReqCallback<string>): void;
@@ -207,7 +207,7 @@ declare namespace InternalFSBinding {
 functionunlink(path: string,usePromises: typeofkUsePromises): Promise<void>;
 
 functionutimes(path: string,atime: number,mtime: number,req: FSReqCallback): void;
-functionutimes(path: string,atime: number,mtime: number,req: undefined,ctx: FSSyncContext): void;
+functionutimes(path: string,atime: number,mtime: number): void;
 functionutimes(path: string,atime: number,mtime: number,usePromises: typeofkUsePromises): Promise<void>;
 
 functionwriteBuffer(fd: number,buffer: ArrayBufferView,offset: number,length: number,position: number|null,req: FSReqCallback<number>): void;