GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — errors: improve performance of NodeError instantiation by Uzlopak · Pull Request #49654 · nodejs/node

Conversation

@Uzlopak

@Uzlopak Uzlopak commented Sep 14, 2023

Copy link
Copy Markdown
Contributor

Well. I dont know why captureLargerStackTrace exists.

Maybe my approach is wrong. I dont know if I lack some node specific knowledge.

I tried to close the gap between the v8 Error class and NodeError. If we really need to support the instantiation of Errors without new, we could wrap it in function. But on the other hand, other Errors are also class type.

The tests basically pass locally, except /test/parallel/test-snapshot-worker.js. Maybe I messed something up (hope not :D)

So please dont be too harsh. 🫣

benchmarks:

Main:

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/node$ node benchmark/error/node-error.js #
error/node-error.js stackTraceLimit=0 code="built-in" n=1000000: 2,740,941.413385955
error/node-error.js stackTraceLimit=10 code="built-in" n=1000000: 530,888.1376037208
error/node-error.js stackTraceLimit=0 code="ERR_HTTP2_STREAM_SELF_DEPENDENCY" n=1000000: 160,106.7862310844
error/node-error.js stackTraceLimit=10 code="ERR_HTTP2_STREAM_SELF_DEPENDENCY" n=1000000: 158,231.65339727345

PR

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/node$ ./node benchmark/error/node-error.js #
error/node-error.js stackTraceLimit=0 code="built-in" n=1000000: 2,755,590.7105245455
error/node-error.js stackTraceLimit=10 code="built-in" n=1000000: 517,735.4538075898
error/node-error.js stackTraceLimit=0 code="ERR_HTTP2_STREAM_SELF_DEPENDENCY" n=1000000: 2,243,410.1895228666
error/node-error.js stackTraceLimit=10 code="ERR_HTTP2_STREAM_SELF_DEPENDENCY" n=1000000: 413,506.70459812187

So before we have about 170k and with the PR we have 413k. So we gain what? 240%?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss, tonivj5, daniellockyer, and Fdawgs reacted with thumbs up emoji bricss, mcollina, debadree25, tonivj5, and metcoder95 reacted with hooray emoji bricss and tonivj5 reacted with heart emoji bricss, tonivj5, H4ad, MoLow, theoludwig, and daniellockyer reacted with rocket emoji
All reactions
  • 👍 4 reactions
  • 🎉 5 reactions
  • ❤️ 2 reactions
  • 🚀 6 reactions
@nodejs-github-bot

nodejs-github-bot commented Sep 14, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/loaders
  • @nodejs/streams
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 14, 2023
@aduh95

aduh95 commented Sep 14, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1386/

Results
                                                                                         confidence improvement accuracy (*)    (**)   (***)
error/error.js n=10000000                                                                               -0.66 %       ±1.20%  ±1.59%  ±2.08%
error/hidestackframes.js n=100000 type='direct-call-noerr'                                               2.16 %       ±4.40%  ±5.86%  ±7.62%
error/hidestackframes.js n=100000 type='direct-call-throw'                                      ***    105.53 %       ±3.23%  ±4.31%  ±5.63%
error/hidestackframes.js n=100000 type='hide-stackframes-noerr'                                          1.63 %       ±5.28%  ±7.03%  ±9.17%
error/hidestackframes.js n=100000 type='hide-stackframes-throw'                                 ***    102.13 %       ±4.01%  ±5.36%  ±7.03%
error/node-error.js stackTraceLimit=0 code='built-in' n=1000000                                   *      3.45 %       ±3.24%  ±4.32%  ±5.63%
error/node-error.js stackTraceLimit=0 code='ERR_HTTP2_STREAM_SELF_DEPENDENCY' n=1000000         ***   1505.54 %      ±38.12% ±51.37% ±68.18%
error/node-error.js stackTraceLimit=10 code='built-in' n=1000000                                        -1.31 %       ±1.80%  ±2.40%  ±3.12%
error/node-error.js stackTraceLimit=10 code='ERR_HTTP2_STREAM_SELF_DEPENDENCY' n=1000000        ***    201.56 %       ±3.60%  ±4.81%  ±6.29%
MoLow reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Uzlopak

Uzlopak commented Sep 14, 2023

Copy link
Copy Markdown
Contributor Author

Do the tests need to pass to trigger the benchmark?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rluvaton

rluvaton commented Sep 14, 2023

Copy link
Copy Markdown
Member

Do the tests need to pass to trigger the benchmark?

no, it's a separate process

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rluvaton rluvaton added performance Issues and PRs related to the performance of Node.js. needs-benchmark-ci PRs that need a benchmark CI run. labels Sep 14, 2023
@Uzlopak

This comment was marked as outdated.

Sign in to view
rluvaton
rluvaton reviewed Sep 14, 2023
View reviewed changes
Comment thread lib/internal/fs/streams.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

mcollina
mcollina approved these changes Sep 15, 2023
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, looks really good

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MoLow reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@Uzlopak

Uzlopak commented Sep 15, 2023

Copy link
Copy Markdown
Contributor Author

I had to patch the wpt testharness, because it checks if the constructor is equal the BaseClass. I tried to figure out if I somehow could cheat the logic. It worked in a user land script but not with the NodeError in NodeJs.
So i dont know if this is a break dealer, or not. I could propose a PR for the wpt testharness repo, but it seems to have alot of PRs unprocessed and if we dont have somebody on the wpr project it wouldnt get merged soon.

I worked 2 days on this PR and I tested a lot of variations on how to get it with prototype inheritance, avoid the ObjectDefineProperties and keep the attributes non-enumarable. But no chance. Only by using the proposed ES6 class I could achieve the desires behaviour.

Regarding the remaining error I really have no clue why it does not create the desired error message. I am really confused. So this why i also proposed the PR, because I a have no clue what the issue is. I thought maybe I messed up hard and somehow share resources between the errors. But no. So if somebody could help with this, because I really dont know what the issue is.

By using an es6 class we use v8s own captureStackTrace Logic and hide the Error with it.

It would also mean, that if we use this solution for all errors of node, that we optimize it basically to the maximum. So for more performance we need then to optimize v8.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rluvaton

rluvaton commented Sep 15, 2023

Copy link
Copy Markdown
Member

Benchmark CI output:

00:54:38.751                                                                                          confidence improvement accuracy (*)    (**)   (***)
00:54:38.751 error/error.js n=10000000                                                                               -0.66 %       ±1.20%  ±1.59%  ±2.08%
00:54:38.751 error/hidestackframes.js n=100000 type='direct-call-noerr'                                               2.16 %       ±4.40%  ±5.86%  ±7.62%
00:54:38.751 error/hidestackframes.js n=100000 type='direct-call-throw'                                      ***    105.53 %       ±3.23%  ±4.31%  ±5.63%
00:54:38.751 error/hidestackframes.js n=100000 type='hide-stackframes-noerr'                                          1.63 %       ±5.28%  ±7.03%  ±9.17%
00:54:38.751 error/hidestackframes.js n=100000 type='hide-stackframes-throw'                                 ***    102.13 %       ±4.01%  ±5.36%  ±7.03%
00:54:38.751 error/node-error.js stackTraceLimit=0 code='built-in' n=1000000                                   *      3.45 %       ±3.24%  ±4.32%  ±5.63%
00:54:38.751 error/node-error.js stackTraceLimit=0 code='ERR_HTTP2_STREAM_SELF_DEPENDENCY' n=1000000         ***   1505.54 %      ±38.12% ±51.37% ±68.18%
00:54:38.751 error/node-error.js stackTraceLimit=10 code='built-in' n=1000000                                        -1.31 %       ±1.80%  ±2.40%  ±3.12%
00:54:38.751 error/node-error.js stackTraceLimit=10 code='ERR_HTTP2_STREAM_SELF_DEPENDENCY' n=1000000        ***    201.56 %       ±3.60%  ±4.81%  ±6.29%
00:54:38.751 
00:54:38.752 Be aware that when doing many comparisons the risk of a false-positive
00:54:38.752 result increases. In this case, there are 9 comparisons, you can thus
00:54:38.752 expect the following amount of false-positive results:
00:54:38.752   0.45 false positives, when considering a   5% risk acceptance (*, **, ***),
00:54:38.752   0.09 false positives, when considering a   1% risk acceptance (**, ***),
00:54:38.752   0.01 false positives, when considering a 0.1% risk acceptance (***)

well done

mcollina, tonivj5, H4ad, Uzlopak, and MoLow reacted with heart emoji
All reactions
  • ❤️ 5 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95
aduh95 reviewed Sep 15, 2023
View reviewed changes
Comment thread lib/internal/errors.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/fixtures/wpt/resources/testharness.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 15, 2023
anonrig
anonrig approved these changes Sep 15, 2023
View reviewed changes
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 15, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 15, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53973/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@benjamingr

benjamingr commented Sep 15, 2023

Copy link
Copy Markdown
Member

Note this means Node gives worse errors since captureLargerStackTrace was used to add more stack frames in case Error.stackTraceLimit was set to a low value (which likely means the user would get no meaningful frames from outside of core) I think?

I think 656ce92#diff-670bf55805b781d9a3579f8bca9104c04d94af87cc33220149fd7d37b095ca1cR413 is the change that added it #35644

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Uzlopak

Uzlopak commented Sep 16, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@benjamingr

My assessment:
I think the reason why we needed captureLargerStackTrace was because the original NodeError function would instantiate first the (base) Error and add the properties to it and return the Error. So when you called e.g. throw new ERR_INVALID_ARG_TYPE you would always have also the NodeError function call in your stack trace as the first entry, which we dont want to show in the stacktrace as it is some node internal specific information nobody cares of. So we captured all stack traces and removed basically the top call away and applied the limit.

Now that we dont need to instantiate the base error but directly generate the error instance, the NodeError StackFrame is not created anymore. So we can use the v8 stacktrace functionality directly, without the need to manually limit the stacktrace.

The new logic:

'use strict';

const kIsNodeError = Symbol('kIsNodeError');

function makeNodeErrorWithCode(Base, key) {
    class NodeError extends Base {
      #message = '';
      code = key;
  
      constructor(...args) {
        super();
        this.message = 'bla'
      }
      get ['constructor']() {
        return Base;
      }
  
      get [kIsNodeError]() {
        return true;
      }
  
      get message() {
        return this.#message;
      }
  
      set message(value) {
        this.#message = value;
      }
  
      [Symbol.toStringTag] = 'Error'

      toString() {
        return `${this.name} [${key}]: ${this.message}`;
      }
    }
    Object.setPrototypeOf(NodeError.constructor, Base)
    return NodeError;
  }

const base = TypeError

const Err = makeNodeErrorWithCode(base, 'ERR_INVALID_URL');

function test() {
  try {
    test1()
  } catch (e) {
    console.log(e.stack)
  }
}

function test1() {
  test2()
}

function test2() {
  test3()
}

function test3() {
  test4()
}

function test4() {
  test5()
}

function test5() {
  test6()
}

function test6() {
  test7()
}

function test7() {
  test8()
}

function test8() {
  test9()
}

function test9() {
  test10()
}

function test10() {
  throw new Err()
}

test()
aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/node$ ./node test.js 
TypeError: bla
    at test10 (/home/aras/workspace/node/test.js:89:9)
    at test9 (/home/aras/workspace/node/test.js:85:3)
    at test8 (/home/aras/workspace/node/test.js:81:3)
    at test7 (/home/aras/workspace/node/test.js:77:3)
    at test6 (/home/aras/workspace/node/test.js:73:3)
    at test5 (/home/aras/workspace/node/test.js:69:3)
    at test4 (/home/aras/workspace/node/test.js:65:3)
    at test3 (/home/aras/workspace/node/test.js:61:3)
    at test2 (/home/aras/workspace/node/test.js:57:3)
    at test1 (/home/aras/workspace/node/test.js:53:3)

vs. the old logic

'use strict';

const kIsNodeError = Symbol('kIsNodeError');
function makeNodeErrorWithCode(Base, key) {
  return function NodeError(...args) {
    const limit = Error.stackTraceLimit;
    Error.stackTraceLimit = 0;
    const error = new Base();
    // Reset the limit and setting the name property.
    Error.stackTraceLimit = limit;

    const message = 'bla'
    Object.defineProperties(error, {
      [kIsNodeError]: {
        __proto__: null,
        value: true,
        enumerable: false,
        writable: false,
        configurable: true,
      },
      message: {
        __proto__: null,
        value: message,
        enumerable: false,
        writable: true,
        configurable: true,
      },
      toString: {
        __proto__: null,
        value() {
          return `${this.name} [${key}]: ${this.message}`;
        },
        enumerable: false,
        writable: true,
        configurable: true,
      },
    });

    Error.captureStackTrace(error);
    error.code = key;
    return error;
  };
}

const base = TypeError

const Err = makeNodeErrorWithCode(base, 'ERR_INVALID_URL');

function test() {
  try {
    test1()
  } catch (e) {
    console.log(e.stack)
  }
}

function test1() {
  test2()
}

function test2() {
  test3()
}

function test3() {
  test4()
}

function test4() {
  test5()
}

function test5() {
  test6()
}

function test6() {
  test7()
}

function test7() {
  test8()
}

function test8() {
  test9()
}

function test9() {
  test10()
}

function test10() {
  throw new Err()
}

Error.stackTraceLimit = 10

test()
TypeError: bla
    at new NodeError (/home/aras/workspace/node/test.js:39:11)
    at test10 (/home/aras/workspace/node/test.js:94:9)
    at test9 (/home/aras/workspace/node/test.js:90:3)
    at test8 (/home/aras/workspace/node/test.js:86:3)
    at test7 (/home/aras/workspace/node/test.js:82:3)
    at test6 (/home/aras/workspace/node/test.js:78:3)
    at test5 (/home/aras/workspace/node/test.js:74:3)
    at test4 (/home/aras/workspace/node/test.js:70:3)
    at test3 (/home/aras/workspace/node/test.js:66:3)
    at test2 (/home/aras/workspace/node/test.js:62:3)
benjamingr and H4ad reacted with thumbs up emoji tonivj5 and H4ad reacted with eyes emoji
All reactions
  • 👍 2 reactions
  • 👀 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Qard
Qard approved these changes Sep 16, 2023
View reviewed changes
@Uzlopak

Uzlopak commented Sep 16, 2023

Copy link
Copy Markdown
Contributor Author

@joyeecheung

Can you please help me, regarding this PR? I cant figure out why ERR_NOT_SUPPORTED_IN_SNAPSHOT is not having the correct message.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

joyeecheung
joyeecheung reviewed Sep 16, 2023
View reviewed changes
Comment thread lib/internal/errors.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot mentioned this pull request Sep 17, 2023
CI Reliability 2023-09-17 nodejs/reliability#668
Open
32 tasks
@Uzlopak

Uzlopak commented Sep 17, 2023

Copy link
Copy Markdown
Contributor Author

@joyeecheung

Is this better?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Uzlopak

Uzlopak commented Sep 17, 2023

Copy link
Copy Markdown
Contributor Author

@joyeecheung

Should the other errors also become enumerable message attributes?

ERR_NOT_BUILDING_SNAPSHOT
ERR_DUPLICATE_STARTUP_SNAPSHOT_MAIN_FUNCTION

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot mentioned this pull request Sep 18, 2023
CI Reliability 2023-09-18 nodejs/reliability#669
Open
33 tasks
@H4ad H4ad mentioned this pull request Sep 18, 2023
Class over raw object nodejs/performance#113
Open
@Uzlopak Uzlopak changed the title errors: improve performance of instantiation errors: improve performance of NodeError instantiation Sep 18, 2023
@github-actions github-actions Bot mentioned this pull request Sep 19, 2023
CI Reliability 2023-09-19 nodejs/reliability#670
Open
32 tasks
@Uzlopak
Uzlopak force-pushed the improve-perf-error-creation branch from e4365ae to a3c2d60 Compare September 27, 2023 22:45
@anonrig anonrig added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Sep 27, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 27, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 27, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54307/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@H4ad H4ad mentioned this pull request Sep 28, 2023
Performance of node:fs nodejs/performance#106
Open
@nodejs-github-bot

nodejs-github-bot commented Sep 28, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54318/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mcollina mcollina added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 28, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Sep 28, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 28, 2023

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/49654
✔  Done loading data for nodejs/node/pull/49654
----------------------------------- PR info ------------------------------------
Title      errors: improve performance of NodeError instantiation (#49654)
Author     Aras Abbasi  (@Uzlopak)
Branch     Uzlopak:improve-perf-error-creation -> nodejs:main
Labels     lib / src, performance, needs-ci, needs-benchmark-ci, commit-queue-squash
Commits    1
 - errors: improve performance of instantiation
Committers 1
 - uzlopak 
PR-URL: https://github.com/nodejs/node/pull/49654
Reviewed-By: Matteo Collina 
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Stephen Belanger 
Reviewed-By: Joyee Cheung 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/49654
Reviewed-By: Matteo Collina 
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Stephen Belanger 
Reviewed-By: Joyee Cheung 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - errors: improve performance of instantiation
   ℹ  This PR was created on Thu, 14 Sep 2023 19:12:31 GMT
   ✔  Approvals: 4
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/49654#pullrequestreview-1628257155
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/49654#pullrequestreview-1642822449
   ✔  - Stephen Belanger (@Qard): https://github.com/nodejs/node/pull/49654#pullrequestreview-1633759369
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/49654#pullrequestreview-1640186686
   ✔  Last GitHub CI successful
   ℹ  Last Benchmark CI on 2023-09-25T21:46:15Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1419/
   ℹ  Last Full PR CI on 2023-09-28T09:00:48Z: https://ci.nodejs.org/job/node-test-pull-request/54318/
- Querying data for job/node-test-pull-request/54318/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/6337251698
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rluvaton
rluvaton approved these changes Sep 28, 2023
View reviewed changes
@rluvaton rluvaton added commit-queue PRs queued for automated landing through the Commit Queue. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Sep 28, 2023
@Uzlopak

Uzlopak commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

Ah come on...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 28, 2023
@nodejs-github-bot
nodejs-github-bot merged commit cc725a6 into nodejs:main Sep 28, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 28, 2023

Copy link
Copy Markdown
Collaborator

Landed in cc725a6

Qard reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Uzlopak
Uzlopak deleted the improve-perf-error-creation branch September 28, 2023 09:58
@Uzlopak

Uzlopak commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

YEAH

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rluvaton

rluvaton commented Sep 28, 2023

Copy link
Copy Markdown
Member

well done @Uzlopak !

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Uzlopak

Uzlopak commented Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

Thank you. Next step is improving perf of SystemError

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@H4ad H4ad mentioned this pull request Sep 29, 2023
Node errors are very slow to create nodejs/performance#40
Open
@RafaelGSS RafaelGSS mentioned this pull request Oct 9, 2023
2023-10-17, Version 21.0.0 (Current) #49870
Merged
targos pushed a commit that referenced this pull request Nov 11, 2023
@Uzlopak @targos
errors: improve performance of instantiation
c71e548
PR-URL: #49654
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
@targos targos mentioned this pull request Nov 12, 2023
v20.10.0 release proposal #50682
Merged
@juanarbol juanarbol mentioned this pull request Dec 6, 2023
2023-12-06, N|Solid v5.0.1 Iron Release nodesource/nsolid#41
Merged
@richardlau richardlau mentioned this pull request Mar 18, 2024
stream: add new when constructing ERR_MULTIPLE_CALLBACK #52110
Merged
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
@Uzlopak @debadree25
errors: improve performance of instantiation
9e04f2c
PR-URL: nodejs#49654
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95 aduh95 aduh95 left review comments
@RafaelGSS RafaelGSS RafaelGSS left review comments
@mcollina mcollina mcollina approved these changes
@Qard Qard Qard approved these changes
@anonrig anonrig anonrig approved these changes
@joyeecheung joyeecheung joyeecheung approved these changes
@rluvaton rluvaton rluvaton approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

commit-queue-squash PRs the Commit Queue should land as one squashed commit. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-benchmark-ci PRs that need a benchmark CI run. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

10 participants

@Uzlopak @nodejs-github-bot @aduh95 @rluvaton @benjamingr @RafaelGSS @anonrig @mcollina @Qard @joyeecheung
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.