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

perf(plugin/prometheus): generate metrics output data with string.buffer #11065

Merged
merged 15 commits into from
Jun 25, 2023

Conversation

chronolaw
Copy link
Contributor

@chronolaw chronolaw commented Jun 14, 2023

Summary

This PR follows #11040, continue to optimize the string usage by string.buffer.

KAG-1852

Checklist

Full changelog

  • use table.new for seen_metrics
  • buffered_print() outputs a string, not a table
  • change printable_metric_data of stream to fit the new buffered_print()
  • do not use ipairs to iterate array
  • other small optimizations

@pull-request-size pull-request-size bot added size/M and removed size/S labels Jun 14, 2023
@chronolaw chronolaw marked this pull request as ready for review June 14, 2023 03:25
@chronolaw
Copy link
Contributor Author

@oowl , could you do the same performance test for this PR again? thanks.

@chronolaw chronolaw requested a review from oowl June 25, 2023 01:50
Copy link
Member

@oowl oowl left a comment

Choose a reason for hiding this comment

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

LGTM, I think maybe we can contribute these performances related patches to https://github.com/knyar/nginx-lua-prometheus upstream, because prometheus.lua was a fork with upstream.

@oowl oowl force-pushed the perf/output_string_buffer_for_prometheus branch from 3f34eac to 4489931 Compare June 25, 2023 03:43
@oowl
Copy link
Member

oowl commented Jun 25, 2023

@oowl , could you do the same performance test for this PR again? thanks.

Sorry, performance test environment was destroy in last month

@chronolaw
Copy link
Contributor Author

LGTM, I think maybe we can contribute these performances related patches to https://github.com/knyar/nginx-lua-prometheus upstream, because prometheus.lua was a fork with upstream.

Sure, Do we have someone responsible for tracking or syncing the nginx-lua-prometheus upstream?

@windmgc windmgc merged commit 027d646 into master Jun 25, 2023
@windmgc windmgc deleted the perf/output_string_buffer_for_prometheus branch June 25, 2023 05:11
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