fs: makes existsSync stop using errors#24068
Closed
arcanis wants to merge 1 commit intonodejs:masterfrom
Closed
Conversation
The previous implementation of `existsSync` was a try/catch on top of `accessSync`. While conceptually sound it was a performance problem when running it repeatedly on non-existing files, because `accessSync` had to create an `Error` object that was immediatly discarded (because `existsSync` never reports anything else than `true` / `false`). This implementation simply checks whether the context would have caused an exception to be thrown, but doesn't actually create it. Fixes: nodejs#24008
addaleax
approved these changes
Nov 3, 2018
Contributor
|
I believe this change is already being made in #24015? |
Contributor
Author
|
Oh, my bad! I see Github still doesn't send notifs when a PR is attached to an issue .. 😅 |
Contributor
I wouldn't worry about that. Hope to see you contribute more in the future. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The previous implementation of
existsSyncwas a try/catch on topof
accessSync. While conceptually sound it was a performance problemwhen running it repeatedly on non-existing files, because
accessSynchad to create an
Errorobject that was immediatly discarded (becauseexistsSyncnever reports anything else thantrue/false).This implementation simply checks whether the context would have caused
an exception to be thrown, but doesn't actually create it.
I benchmarked it as such:
Fixes: #24008
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes