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

Update performance benchmarks for v2 #163

Merged
merged 3 commits into from
Mar 13, 2023
Merged

Update performance benchmarks for v2 #163

merged 3 commits into from
Mar 13, 2023

Conversation

minrk
Copy link
Member

@minrk minrk commented Feb 24, 2023

I'm trying to get the benchmarks to run again so we can evaluate our current performance situation. But it's currently failing in most cases.

I've found a few performance things we can improve:

  • use single endpoint to check for availability, which is far faster and cheaper than requesting all endpoints
  • add concurrency limits to avoid timeouts due to the event-loop being busy rather than the specific action taking a long time (CHP client already has similar for the same reason)

I'm still running into cannot assign requested address problems with consul, which appear to be failures in aiohttp? I know that's been an issue with aiohttp in the past, but haven't identified what can be done about it.

placeholder PR for now while I'm making a mess, I'll add a comment when I think this is worth looking at.

@minrk
Copy link
Member Author

minrk commented Mar 3, 2023

This now all works, just breaking up the changes into clearer PRs:

I'll rebase this PR purely with the benchmark updates after those are merged.

@minrk
Copy link
Member Author

minrk commented Mar 3, 2023

I re-ran the benchmarks on a big VM, and the important thing is that with the changes here and in traefik v2, we no longer have an O(routes) problem, and performance is consistent and acceptable for up to 500 routes, which was the main thing we were after! #165 was critical for getting the benchmarks to run at all, and setting traefik.providersThrottleDuration to '0s' by default, we see performance much closer to CHP (still, CHP is closer to 1ms while traefik is closer to 100ms, but that's fast enough for us, where before it was closer to 2s because of the throttling)

@minrk minrk changed the title [WIP] performance improvements, benchmarks Update performance benchmarks for v2 Mar 10, 2023
@minrk minrk marked this pull request as ready for review March 10, 2023 19:09
@minrk
Copy link
Member Author

minrk commented Mar 10, 2023

@GeorgianaElena Now that all the actual changes have been merged, this is purely an update to the benchmarks themselves, and a re-run of the data, updating the notebook.

Copy link
Member

@manics manics left a comment

Choose a reason for hiding this comment

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

I think it'd be helpful to have a short performance/README.md explaining where to start (probably just run <these-scripts>)

@minrk
Copy link
Member Author

minrk commented Mar 13, 2023

@manics good call. Added a README

Copy link
Member

@GeorgianaElena GeorgianaElena left a comment

Choose a reason for hiding this comment

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

🎉 🎉 🎉

@minrk minrk merged commit 4a2750a into jupyterhub:main Mar 13, 2023
@minrk minrk deleted the perf branch March 13, 2023 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants