fs: improve error performance of chownSync

PR-URL: #49962
Refs: nodejs/performance#106
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>

Author: anonrig

benchmark/fs/bench-chownSync.js

@@ -0,0 +1,54 @@
+'use strict';
+
+constcommon=require('../common');
+constfs=require('fs');
+constassert=require('assert');
+consttmpdir=require('../../test/common/tmpdir');
+
+if(process.platform==='win32'){
+console.log('Skipping: Windows does not have `getuid` or `getgid`');
+process.exit(0);
+}
+
+constbench=common.createBenchmark(main,{
+type: ['existing','non-existing'],
+method: ['chownSync','lchownSync'],
+n: [1e4],
+});
+
+functionmain({ n, type, method }){
+constuid=process.getuid();
+constgid=process.getgid();
+constfsMethod=fs[method];
+
+switch(type){
+case'existing': {
+tmpdir.refresh();
+consttmpfile=tmpdir.resolve(`.existing-file-${process.pid}`);
+fs.writeFileSync(tmpfile,'this-is-for-a-benchmark','utf8');
+bench.start();
+for(leti=0;i<n;i++){
+fsMethod(tmpfile,uid,gid);
+}
+bench.end(n);
+break;
+}
+case'non-existing': {
+constpath=tmpdir.resolve(`.non-existing-file-${Date.now()}`);
+lethasError=false;
+bench.start();
+for(leti=0;i<n;i++){
+try{
+fs[method](path,uid,gid);
+}catch{
+hasError=true;
+}
+}
+bench.end(n);
+assert(hasError);
+break;
+}
+default:
+newError('Invalid type');
+}
+}

lib/fs.js

@@ -2089,9 +2089,11 @@ function chownSync(path, uid, gid) {
 path=getValidatedPath(path);
 validateInteger(uid,'uid',-1,kMaxUserId);
 validateInteger(gid,'gid',-1,kMaxUserId);
-constctx={ path };
-binding.chown(pathModule.toNamespacedPath(path),uid,gid,undefined,ctx);
-handleErrorFromBinding(ctx);
+binding.chown(
+pathModule.toNamespacedPath(path),
+uid,
+gid,
+);
 }
 
 /**

src/node_file.cc

@@ -2572,18 +2572,16 @@ static void Chown(const FunctionCallbackInfo<Value>& args) {
 CHECK(IsSafeJsInt(args[2]));
 constuv_gid_t gid = static_cast<uv_gid_t>(args[2].As<Integer>()->Value());
 
-FSReqBase* req_wrap_async = GetReqWrap(args, 3);
-if (req_wrap_async != nullptr) { // chown(path, uid, gid, req)
+if (argc > 3) { // chown(path, uid, gid, req)
+ FSReqBase* req_wrap_async = GetReqWrap(args, 3);
 FS_ASYNC_TRACE_BEGIN1(
 UV_FS_CHOWN, req_wrap_async, "path", TRACE_STR_COPY(*path))
 AsyncCall(env, req_wrap_async, args, "chown", UTF8, AfterNoArgs,
  uv_fs_chown, *path, uid, gid);
- } else { // chown(path, uid, gid, undefined, ctx)
-CHECK_EQ(argc, 5);
- FSReqWrapSync req_wrap_sync;
+ } else { // chown(path, uid, gid)
+ FSReqWrapSync req_wrap_sync("chown", *path);
 FS_SYNC_TRACE_BEGIN(chown);
-SyncCall(env, args[4], &req_wrap_sync, "chown",
- uv_fs_chown, *path, uid, gid);
+SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_chown, *path, uid, gid);
 FS_SYNC_TRACE_END(chown);
  }
 }

typings/internalBinding/fs.d.ts

@@ -67,6 +67,7 @@ declare namespace InternalFSBinding {
 functionchown(path: string,uid: number,gid: number,req: FSReqCallback): void;
 functionchown(path: string,uid: number,gid: number,req: undefined,ctx: FSSyncContext): void;
 functionchown(path: string,uid: number,gid: number,usePromises: typeofkUsePromises): Promise<void>;
+functionchown(path: string,uid: number,gid: number): void;
 
 functionclose(fd: number,req: FSReqCallback): void;
 functionclose(fd: number,req: undefined,ctx: FSSyncContext): void;