Skip to content
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

Move the huffman benchmark base64 out of init #262

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Nov 25, 2020

Motivation:

The init() of all benchmarks will execute, even when we filter a
benchmark run down to a single benchmark. We should avoid doing heavy
work there, because it can confuse us when we look at traces elsewhere.

Modifications:

  • Move some heavy work in the huffman benchmark base64 decode out of
    init().
  • Prevent setUp() and tearDown() from running if a test is filtered out.
  • Avoid even creating benchmarks unless they're filtered in.

Result:

Less confusing benchmark traces.

Motivation:

The init() of all benchmarks will execute, even when we filter a
benchmark run down to a single benchmark. We should avoid doing heavy
work there, because it can confuse us when we look at traces elsewhere.

Modifications:

- Move some heavy work in the huffman benchmark base64 decode out of
  init().
- Prevent setUp() and tearDown() from running if a test is filtered out.
- Avoid even creating benchmarks unless they're filtered in.

Result:

Less confusing benchmark traces.
@Lukasa Lukasa added the semver/none No version bump required. label Nov 25, 2020
@Lukasa
Copy link
Contributor Author

Lukasa commented Nov 25, 2020

As a note: in our case, without the base64 jazz needed to set up the HPACK cases, we see a 17% speedup in the 100 concurrent streams benchmark. We were wasting so much time doing base64 nonsense in these benchmarks. All prior speedup results I've quoted in the past few days should be proportionally increased to account for the fact that they were swamped by this nonsense work we shouldn't have been doing.

@PeterAdams-A PeterAdams-A merged commit ec1fad4 into apple:main Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/none No version bump required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants