GeezerVison is showing github.com — fs: improve error performance of `fs.dir` · nodejs/node@128e514
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/fs/dir.js +8-17Lines changed: 8 additions & 17 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -26,7 +26,6 @@ const { | 26 | 26 | getDirent, | 27 | 27 | getOptions, | 28 | 28 | getValidatedPath, | 29 | | - handleErrorFromBinding, | 30 | 29 | } = require('internal/fs/utils'); | 31 | 30 | const { | 32 | 31 | validateFunction, | @@ -155,27 +154,26 @@ class Dir { | 155 | 154 |
| 156 | 155 | readSyncRecursive(dirent) { | 157 | 156 | const path = pathModule.join(dirent.parentPath, dirent.name); | 158 | | - const ctx = { path }; | 159 | 157 | const handle = dirBinding.opendir( | 160 | 158 | pathModule.toNamespacedPath(path), | 161 | 159 | this.#options.encoding, | 162 | | - undefined, | 163 | | - ctx, | 164 | 160 | ); | 165 | | - handleErrorFromBinding(ctx); | | 161 | + | | 162 | + // Terminate early, since it's already thrown. | | 163 | + if (handle === undefined) { | | 164 | + return; | | 165 | + } | | 166 | + | 166 | 167 | const result = handle.read( | 167 | 168 | this.#options.encoding, | 168 | 169 | this.#options.bufferSize, | 169 | | - undefined, | 170 | | - ctx, | 171 | 170 | ); | 172 | 171 |
| 173 | 172 | if (result) { | 174 | 173 | this.processReadResult(path, result); | 175 | 174 | } | 176 | 175 |
| 177 | | - handle.close(undefined, ctx); | 178 | | - handleErrorFromBinding(ctx); | | 176 | + handle.close(); | 179 | 177 | } | 180 | 178 |
| 181 | 179 | readSync() { | @@ -195,14 +193,10 @@ class Dir { | 195 | 193 | return dirent; | 196 | 194 | } | 197 | 195 |
| 198 | | - const ctx = { path: this.#path }; | 199 | 196 | const result = this.#handle.read( | 200 | 197 | this.#options.encoding, | 201 | 198 | this.#options.bufferSize, | 202 | | - undefined, | 203 | | - ctx, | 204 | 199 | ); | 205 | | - handleErrorFromBinding(ctx); | 206 | 200 |
| 207 | 201 | if (result === null) { | 208 | 202 | return result; | @@ -257,10 +251,7 @@ class Dir { | 257 | 251 | } | 258 | 252 |
| 259 | 253 | this.#closed = true; | 260 | | - const ctx = { path: this.#path }; | 261 | | - const result = this.#handle.close(undefined, ctx); | 262 | | - handleErrorFromBinding(ctx); | 263 | | - return result; | | 254 | + this.#handle.close(); | 264 | 255 | } | 265 | 256 |
| 266 | 257 | async* entries() { | |
Collapse file Copy file name to clipboardExpand all lines: src/node_dir.cc +30-34Lines changed: 30 additions & 34 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 3 | 3 | #include "node_external_reference.h" | 4 | 4 | #include "node_file-inl.h" | 5 | 5 | #include "node_process-inl.h" | | 6 | +#include "path.h" | 6 | 7 | #include "permission/permission.h" | 7 | 8 | #include "util.h" | 8 | 9 |
| @@ -183,26 +184,24 @@ void AfterClose(uv_fs_t* req) { | 183 | 184 | void DirHandle::Close(const FunctionCallbackInfo<Value>& args) { | 184 | 185 | Environment* env = Environment::GetCurrent(args); | 185 | 186 |
| 186 | | - const int argc = args.Length(); | 187 | | - CHECK_GE(argc, 1); | | 187 | + CHECK_GE(args.Length(), 0); // [req] | 188 | 188 |
| 189 | 189 | DirHandle* dir; | 190 | 190 | ASSIGN_OR_RETURN_UNWRAP(&dir, args.This()); | 191 | 191 |
| 192 | 192 | dir->closing_ = false; | 193 | 193 | dir->closed_ = true; | 194 | 194 |
| 195 | | - FSReqBase* req_wrap_async = GetReqWrap(args, 0); | 196 | | - if (req_wrap_async != nullptr) { // close(req) | | 195 | + if (!args[0]->IsUndefined()) { // close(req) | | 196 | + FSReqBase* req_wrap_async = GetReqWrap(args, 0); | | 197 | + CHECK_NOT_NULL(req_wrap_async); | 197 | 198 | FS_DIR_ASYNC_TRACE_BEGIN0(UV_FS_CLOSEDIR, req_wrap_async) | 198 | 199 | AsyncCall(env, req_wrap_async, args, "closedir", UTF8, AfterClose, | 199 | 200 | uv_fs_closedir, dir->dir()); | 200 | | - } else { // close(undefined, ctx) | 201 | | - CHECK_EQ(argc, 2); | 202 | | - FSReqWrapSync req_wrap_sync; | | 201 | + } else { // close() | | 202 | + FSReqWrapSync req_wrap_sync("closedir"); | 203 | 203 | FS_DIR_SYNC_TRACE_BEGIN(closedir); | 204 | | - SyncCall(env, args[1], &req_wrap_sync, "closedir", uv_fs_closedir, | 205 | | - dir->dir()); | | 204 | + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_closedir, dir->dir()); | 206 | 205 | FS_DIR_SYNC_TRACE_END(closedir); | 207 | 206 | } | 208 | 207 | } | @@ -282,8 +281,7 @@ void DirHandle::Read(const FunctionCallbackInfo<Value>& args) { | 282 | 281 | Environment* env = Environment::GetCurrent(args); | 283 | 282 | Isolate* isolate = env->isolate(); | 284 | 283 |
| 285 | | - const int argc = args.Length(); | 286 | | - CHECK_GE(argc, 3); | | 284 | + CHECK_GE(args.Length(), 2); // encoding, bufferSize, [callback] | 287 | 285 |
| 288 | 286 | const enum encoding encoding = ParseEncoding(isolate, args[0], UTF8); | 289 | 287 |
| @@ -299,27 +297,25 @@ void DirHandle::Read(const FunctionCallbackInfo<Value>& args) { | 299 | 297 | dir->dir_->dirents = dir->dirents_.data(); | 300 | 298 | } | 301 | 299 |
| 302 | | - FSReqBase* req_wrap_async = GetReqWrap(args, 2); | 303 | | - if (req_wrap_async != nullptr) { // dir.read(encoding, bufferSize, req) | | 300 | + if (!args[2]->IsUndefined()) { // dir.read(encoding, bufferSize, req) | | 301 | + FSReqBase* req_wrap_async = GetReqWrap(args, 2); | | 302 | + CHECK_NOT_NULL(req_wrap_async); | 304 | 303 | FS_DIR_ASYNC_TRACE_BEGIN0(UV_FS_READDIR, req_wrap_async) | 305 | 304 | AsyncCall(env, req_wrap_async, args, "readdir", encoding, | 306 | 305 | AfterDirRead, uv_fs_readdir, dir->dir()); | 307 | | - } else { // dir.read(encoding, bufferSize, undefined, ctx) | 308 | | - CHECK_EQ(argc, 4); | 309 | | - FSReqWrapSync req_wrap_sync; | | 306 | + } else { // dir.read(encoding, bufferSize) | | 307 | + FSReqWrapSync req_wrap_sync("readdir"); | 310 | 308 | FS_DIR_SYNC_TRACE_BEGIN(readdir); | 311 | | - int err = SyncCall(env, args[3], &req_wrap_sync, "readdir", uv_fs_readdir, | 312 | | - dir->dir()); | | 309 | + int err = | | 310 | + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_readdir, dir->dir()); | 313 | 311 | FS_DIR_SYNC_TRACE_END(readdir); | 314 | 312 | if (err < 0) { | 315 | | - return; // syscall failed, no need to continue, error info is in ctx | | 313 | + return; // syscall failed, no need to continue, error is already thrown | 316 | 314 | } | 317 | 315 |
| 318 | 316 | if (req_wrap_sync.req.result == 0) { | 319 | 317 | // Done | 320 | | - Local<Value> done = Null(isolate); | 321 | | - args.GetReturnValue().Set(done); | 322 | | - return; | | 318 | + return args.GetReturnValue().SetNull(); | 323 | 319 | } | 324 | 320 |
| 325 | 321 | CHECK_GE(req_wrap_sync.req.result, 0); | @@ -332,8 +328,9 @@ void DirHandle::Read(const FunctionCallbackInfo<Value>& args) { | 332 | 328 | encoding, | 333 | 329 | &error) | 334 | 330 | .ToLocal(&js_array)) { | 335 | | - Local<Object> ctx = args[2].As<Object>(); | 336 | | - USE(ctx->Set(env->context(), env->error_string(), error)); | | 331 | + // TODO(anonrig): Initializing BufferValue here is wasteful. | | 332 | + BufferValue error_payload(isolate, error); | | 333 | + env->ThrowError(error_payload.out()); | 337 | 334 | return; | 338 | 335 | } | 339 | 336 |
| @@ -362,16 +359,16 @@ static void OpenDir(const FunctionCallbackInfo<Value>& args) { | 362 | 359 | Environment* env = Environment::GetCurrent(args); | 363 | 360 | Isolate* isolate = env->isolate(); | 364 | 361 |
| 365 | | - const int argc = args.Length(); | 366 | | - CHECK_GE(argc, 3); | | 362 | + CHECK_GE(args.Length(), 2); // path, encoding, [callback] | 367 | 363 |
| 368 | 364 | BufferValue path(isolate, args[0]); | 369 | 365 | CHECK_NOT_NULL(*path); | 370 | 366 |
| 371 | 367 | const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); | 372 | 368 |
| 373 | | - FSReqBase* req_wrap_async = GetReqWrap(args, 2); | 374 | | - if (req_wrap_async != nullptr) { // openDir(path, encoding, req) | | 369 | + if (!args[2]->IsUndefined()) { // openDir(path, encoding, req) | | 370 | + FSReqBase* req_wrap_async = GetReqWrap(args, 2); | | 371 | + CHECK_NOT_NULL(req_wrap_async); | 375 | 372 | ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( | 376 | 373 | env, | 377 | 374 | req_wrap_async, | @@ -381,17 +378,16 @@ static void OpenDir(const FunctionCallbackInfo<Value>& args) { | 381 | 378 | UV_FS_OPENDIR, req_wrap_async, "path", TRACE_STR_COPY(*path)) | 382 | 379 | AsyncCall(env, req_wrap_async, args, "opendir", encoding, AfterOpenDir, | 383 | 380 | uv_fs_opendir, *path); | 384 | | - } else { // openDir(path, encoding, undefined, ctx) | 385 | | - CHECK_EQ(argc, 4); | | 381 | + } else { // openDir(path, encoding) | 386 | 382 | THROW_IF_INSUFFICIENT_PERMISSIONS( | 387 | 383 | env, permission::PermissionScope::kFileSystemRead, path.ToStringView()); | 388 | | - FSReqWrapSync req_wrap_sync; | | 384 | + FSReqWrapSync req_wrap_sync("opendir", *path); | 389 | 385 | FS_DIR_SYNC_TRACE_BEGIN(opendir); | 390 | | - int result = SyncCall(env, args[3], &req_wrap_sync, "opendir", | 391 | | - uv_fs_opendir, *path); | | 386 | + int result = | | 387 | + SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_opendir, *path); | 392 | 388 | FS_DIR_SYNC_TRACE_END(opendir); | 393 | 389 | if (result < 0) { | 394 | | - return; // syscall failed, no need to continue, error info is in ctx | | 390 | + return; // syscall failed, no need to continue, error is already thrown | 395 | 391 | } | 396 | 392 |
| 397 | 393 | uv_fs_t* req = &req_wrap_sync.req; | |
Collapse file Copy file name to clipboardExpand all lines: typings/internalBinding/fs_dir.d.ts +5-3Lines changed: 5 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -6,12 +6,14 @@ declare namespace InternalFSDirBinding { | 6 | 6 | type StringOrBuffer = string | Buffer; | 7 | 7 |
| 8 | 8 | class DirHandle { | 9 | | - read(encoding: string, bufferSize: number, _: unknown, ctx: ReadFileContext): string[] | undefined; | 10 | | - close(_: unknown, ctx: ReadFileContext): void; | | 9 | + read(encoding: string, bufferSize: number, callback: FSReqCallback): string[] | undefined; | | 10 | + read(encoding: string, bufferSize: number): string[] | undefined; | | 11 | + close(callback: FSReqCallback): void; | | 12 | + close(): void; | 11 | 13 | } | 12 | 14 |
| 13 | 15 | function opendir(path: StringOrBuffer, encoding: string, req: FSReqCallback): DirHandle; | 14 | | - function opendir(path: StringOrBuffer, encoding: string, _: undefined, ctx: ReadFileContext): DirHandle; | | 16 | + function opendir(path: StringOrBuffer, encoding: string): DirHandle; | 15 | 17 | function opendirSync(path: StringOrBuffer): DirHandle; | 16 | 18 | } | 17 | 19 |
| |
|