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

Improve performance of registry.metrics() #276

Closed

Conversation

SerayaEryn
Copy link
Contributor

This improves the performance of registry.metrics() by approximately 40%.
This is achieved by creating a specific formatter for each metric.

Here are the benchmark results:

### Summary:

⚠ registry ➭ getMetricsAsJSON#1 with 64 is 2.343% acceptably slower.
✓ registry ➭ getMetricsAsJSON#2 with 8 is 0.8804% faster.
⚠ registry ➭ getMetricsAsJSON#2 with 4 and 2 with 2 is 1.183% acceptably slower.
✓ registry ➭ getMetricsAsJSON#2 with 2 and 2 with 4 is 0.2041% faster.
✓ registry ➭ getMetricsAsJSON#6 with 2 is 0.7355% faster.
✓ registry ➭ metrics#1 with 64 is 40.57% faster.
✓ registry ➭ metrics#2 with 8 is 38.32% faster.
✓ registry ➭ metrics#2 with 4 and 2 with 2 is 41.24% faster.
✓ registry ➭ metrics#2 with 2 and 2 with 4 is 43.03% faster.
✓ registry ➭ metrics#6 with 2 is 41.95% faster.
✓ histogram ➭ observe#1 with 64 is 4.848% faster.
✓ histogram ➭ observe#2 with 8 is 3.919% faster.
⚠ histogram ➭ observe#2 with 4 and 2 with 2 is 0.5036% acceptably slower.
✓ histogram ➭ observe#2 with 2 and 2 with 4 is 1.179% faster.
✓ histogram ➭ observe#6 with 2 is 11.81% faster.
⚠ summary ➭ observe#1 with 64 is 2.889% acceptably slower.
⚠ summary ➭ observe#2 with 8 is 2.747% acceptably slower.
✓ summary ➭ observe#2 with 4 and 2 with 2 is 2.210% faster.
✓ summary ➭ observe#2 with 2 and 2 with 4 is 0.1158% faster.
⚠ summary ➭ observe#6 with 2 is 3.125% acceptably slower.

@zbjornson
Copy link
Collaborator

Sorry for letting this sit. A lot of similar changes (the extends Metric changes) landed recently but I think the perf changes are still useful. Would you be up for rebasing this on master please?

@doochik
Copy link
Contributor

doochik commented May 14, 2020

@zbjornson Hi!
I've rebased this PR #373

@zbjornson
Copy link
Collaborator

Thanks! will review that one this weekend.

@zbjornson zbjornson closed this May 14, 2020
@zbjornson
Copy link
Collaborator

Oops, I just realized the original author of this PR @SerayaEryn is not @doochik, who rebased it. doochik preserved the authorship of SerayaEryn's commit though so you'll still get attribution. @SerayaEryn alright with you?

@SerayaEryn
Copy link
Contributor Author

Yes, it is alright.

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.

3 participants