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

Add JMH Benchmarks for the critical code paths #1146

Closed
aepfli opened this issue Oct 7, 2024 · 4 comments
Closed

Add JMH Benchmarks for the critical code paths #1146

aepfli opened this issue Oct 7, 2024 · 4 comments
Assignees

Comments

@aepfli
Copy link
Member

aepfli commented Oct 7, 2024

Our tool is excellent, but more and more people are using it. We already received internal information, that some code paths tend to be hotspots, especially structure operations.

Evaluating this without a proper test harness around it is hard; hence, it might be a good idea to add a JMH Suite to our tests for verification and to allow us to spot hotspots earlier in the future.

@toddbaert
Copy link
Member

Yes this is a great idea. We could run these as part of the SDK CI.

We may also want to add something similar to the contribs eventually, though there it will be up to each component owner what constitutes adequate performance.

@toddbaert toddbaert self-assigned this Oct 7, 2024
@toddbaert
Copy link
Member

I'm not sure if #1156 fully resolves this, but it's certainly related.

@aepfli I'm happy to add a check that will check we don't have regressions with this number, but it will involve a bit of a hacky script.

@aepfli
Copy link
Member Author

aepfli commented Oct 16, 2024

Do you think we can close this as #1156 is merged, or do you want to invest further in it? If yes, we should maybe adapt the ticket description to add more context and concrete to-dos.

I should start correctly reading comments. Sorry for this. Well, I am fine for now. Some verification would be good, or some persistence of at least the latest results, so we can at least verify from time to time. Furthermore, it would be cool if we found a tool where we can add this metric correctly to draw nice graphs and see how the performance evolves. But without a proper tool to display the data, I don't see a benefit in adding it to the CI.

@aepfli
Copy link
Member Author

aepfli commented Oct 24, 2024

i think with #1182 and #1178 this can be closed now

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

No branches or pull requests

3 participants