Skip to content

Conversation

addaleax
Copy link
Member

Stack trace capturing currently accounts for 40 % of the benchmark
running time. Always throwing the same exception object removes
that overhead and lets the benchmark be more focused on what it is
supposed to measure.

Refs: #34512 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Stack trace capturing currently accounts for 40 % of the benchmark
running time. Always throwing the same exception object removes
that overhead and lets the benchmark be more focused on what it is
supposed to measure.

Refs: nodejs#34512 (comment)
@nodejs-github-bot nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. benchmark Issues and PRs related to the benchmark subsystem. labels Jul 26, 2020
@jasnell
Copy link
Member

jasnell commented Jul 27, 2020

Hmm, I'm torn on this. While I get the reason for the change, it makes the benchmark even more unrealistic compared to real world user code. Our microbenchmarks are already overly optimized as it is. Not a blocking concern, however.

@addaleax
Copy link
Member Author

@jasnell I mean, that really depends on what we’re measuring here, right? The point is to measure the performance effect of async_hooks on Promises. Let’s leave that out which isn’t relevant to it.

(For example, the other async_hooks benchmarks are very diluted because they’re full-blown HTTP servers – that might be closer to a real-world example, but it makes it harder to measure the perf impact of async_hooks themselves. For profiling, they are basically useless and I always needed to go to the MessagePort benchmarks instead.)

@jasnell
Copy link
Member

jasnell commented Jul 27, 2020

Yeah like I said, I'm torn on it and don't consider it blocking. I just think we way over index on microbenchmarks in general.

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 27, 2020
@nodejs-github-bot
Copy link
Collaborator

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

puzpuzpuz pushed a commit that referenced this pull request Jul 29, 2020
Stack trace capturing currently accounts for 40 % of the benchmark
running time. Always throwing the same exception object removes
that overhead and lets the benchmark be more focused on what it is
supposed to measure.

Refs: #34512 (comment)

PR-URL: #34523
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Andrey Pechkurov <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
@puzpuzpuz
Copy link
Member

Landed in b14ce72

@puzpuzpuz puzpuzpuz closed this Jul 29, 2020
codebytere pushed a commit that referenced this pull request Aug 5, 2020
Stack trace capturing currently accounts for 40 % of the benchmark
running time. Always throwing the same exception object removes
that overhead and lets the benchmark be more focused on what it is
supposed to measure.

Refs: #34512 (comment)

PR-URL: #34523
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Andrey Pechkurov <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
@codebytere codebytere mentioned this pull request Aug 10, 2020
addaleax added a commit that referenced this pull request Sep 22, 2020
Stack trace capturing currently accounts for 40 % of the benchmark
running time. Always throwing the same exception object removes
that overhead and lets the benchmark be more focused on what it is
supposed to measure.

Refs: #34512 (comment)

PR-URL: #34523
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Andrey Pechkurov <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
addaleax added a commit that referenced this pull request Sep 22, 2020
Stack trace capturing currently accounts for 40 % of the benchmark
running time. Always throwing the same exception object removes
that overhead and lets the benchmark be more focused on what it is
supposed to measure.

Refs: #34512 (comment)

PR-URL: #34523
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Andrey Pechkurov <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. benchmark Issues and PRs related to the benchmark subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants