-
-
Notifications
You must be signed in to change notification settings - Fork 36.6k
lib: improve error creation performance #24747
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -261,10 +261,16 @@ function uvException(ctx) { | |
| message += ` -> '${dest}'`; | ||
| } | ||
|
|
||
| // Reducing the limit improves the performance significantly. We do not loose | ||
| // the stack frames due to the `captureStackTrace()` function that is called | ||
| // later. | ||
| const tmpLimit = Error.stackTraceLimit; | ||
| Error.stackTraceLimit = 0; | ||
| // Pass the message to the constructor instead of setting it on the object | ||
| // to make sure it is the same as the one created in C++ | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| const err = new Error(message); | ||
| Error.stackTraceLimit = tmpLimit; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like something V8 folks should be made aware of since, in this case, the lazily populated I'd rather this be tracked as a V8 issue and fixed there than throwing
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned above, I already did that :)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but the V8 issues mentioned above
don't clearly state this specific case though.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened another issue to properly address this: https://bugs.chromium.org/p/v8/issues/detail?id=8555 |
||
|
|
||
| for (const prop of Object.keys(ctx)) { | ||
| if (prop === 'message' || prop === 'path' || prop === 'dest') { | ||
|
|
@@ -307,8 +313,14 @@ function uvExceptionWithHostPort(err, syscall, address, port) { | |
| details = ` ${address}`; | ||
| } | ||
|
|
||
| // Reducing the limit improves the performance significantly. We do not loose | ||
| // the stack frames due to the `captureStackTrace()` function that is called | ||
| // later. | ||
| const tmpLimit = Error.stackTraceLimit; | ||
| Error.stackTraceLimit = 0; | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| const ex = new Error(`${message}${details}`); | ||
| Error.stackTraceLimit = tmpLimit; | ||
| ex.code = code; | ||
| ex.errno = code; | ||
| ex.syscall = syscall; | ||
|
|
@@ -377,9 +389,15 @@ function exceptionWithHostPort(err, syscall, address, port, additional) { | |
| details += ` - Local (${additional})`; | ||
| } | ||
|
|
||
| // Reducing the limit improves the performance significantly. We do not loose | ||
| // the stack frames due to the `captureStackTrace()` function that is called | ||
| // later. | ||
| const tmpLimit = Error.stackTraceLimit; | ||
| Error.stackTraceLimit = 0; | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| const ex = new Error(`${syscall} ${code}${details}`); | ||
| // TODO(joyeecheung): errno is supposed to err, like in uvException | ||
| Error.stackTraceLimit = tmpLimit; | ||
| ex.code = ex.errno = code; | ||
| ex.syscall = syscall; | ||
| ex.address = address; | ||
|
|
@@ -410,9 +428,15 @@ function dnsException(code, syscall, hostname) { | |
| } | ||
| } | ||
| const message = `${syscall} ${code}${hostname ? ` ${hostname}` : ''}`; | ||
| // Reducing the limit improves the performance significantly. We do not loose | ||
| // the stack frames due to the `captureStackTrace()` function that is called | ||
| // later. | ||
| const tmpLimit = Error.stackTraceLimit; | ||
| Error.stackTraceLimit = 0; | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| const ex = new Error(message); | ||
| // TODO(joyeecheung): errno is supposed to be a number / err, like in | ||
| Error.stackTraceLimit = tmpLimit; | ||
| // uvException. | ||
| ex.errno = code; | ||
| ex.code = code; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.