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

Memoize Metric::Capture.capture_cols #15791

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented Aug 11, 2017

capture_cols will always be the same regardless of when it is called, unless a database update happens to the metrics table, in which the process would be restarted after the migration has happened anyway, and it will get reloaded then.

This method is hit frequently on the metrics collectors, so this prevents a bunch of objects from being allocated multiple times when doing perf_capture_realtime:

On a sample benchmark for perf_capture_realtime:

Before 20172 objects allocated from this method alone
After None (after being memoize)

Links

capture_cols will always be the same regardless of when it is called,
unless a database update happens to the metrics table, in which the
process would be restarted after the migration has happened, and it will
get reloaded then.

This method is hit frequently on the metrics collectors, so this
prevents a bunch of objects from being allocated multiple times when
doing perf_capture_realtime:

On a sample benchmark for perf_capture_realtime:

Before:  20172 objects allocated from this method
After:   None (after being memoize)
@miq-bot
Copy link
Member

miq-bot commented Aug 11, 2017

Checked commit NickLaMuro@b78b818 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@Fryguy Fryguy merged commit 3b0cc7a into ManageIQ:master Aug 11, 2017
@Fryguy Fryguy added this to the Sprint 67 Ending Aug 21, 2017 milestone Aug 11, 2017
@Fryguy Fryguy self-assigned this Aug 11, 2017
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