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

WebSocket and logging test maintenance #19

Merged
merged 2 commits into from
Feb 10, 2024

Conversation

awelzel
Copy link
Contributor

@awelzel awelzel commented Feb 8, 2024

The logging micro-benchmark change will skew the results, but unclear how useful the previous once were anyhow. Might make sense to keep a hash of the micro-benchmark file in the results though to detect something like that.

Relates to zeek/zeek#3331

Previously, these benchmarks would run after Zeek has decided to shut down,
making Zeek's internal batching ineffective.
These were created by tunneling various traffic with wstunnel (HTTP
downloads and SSH uploads and a tiny bit of SSL). Hook up the both,
BinPac and Spicy versions to see improvements or regressions in Spicy
over time.

Here these run in 8.3 vs 14.4 seconds runtime, with bare mode taking
some 3.5 seconds to process the pcap.
@awelzel awelzel requested a review from timwoj February 8, 2024 18:08
Copy link
Member

@timwoj timwoj left a comment

Choose a reason for hiding this comment

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

The logging micro-benchmark change will skew the results, but unclear how useful the previous once were anyhow.

I think this is fine because you're typically only comparing short time windows, such as a PR vs a recent master branch. As long as we're getting re-runs of the benchmarks on the various release branches (I think that happens?), the long-term comparisons should be fine also.

@awelzel awelzel merged commit 03b80c3 into master Feb 10, 2024
3 checks passed
@awelzel awelzel deleted the topic/awelzel/websocket-and-maintenance branch February 10, 2024 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants