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

Speed up widget generation #14224

Merged
merged 1 commit into from
Mar 8, 2017
Merged

Speed up widget generation #14224

merged 1 commit into from
Mar 8, 2017

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Mar 7, 2017

Reporting is bringing back all rows to check for data types.
For widgets, this was bringing back unnecessary data.

The change detects if the input is a query, and uses ActiveRecord::Relation#klass instead of fetching all rows and comparing them.

https://bugzilla.redhat.com/show_bug.cgi?id=1251259

ms bytes objects queries query (ms) rows comments
5,962.8 51,785,052* 6,412,174 1,196 1,570.6 6,768 before.4
260.9 6,975,857 84,669 56 105.8 16 after.4
95.6% >87% 99% 95% 93.3% 99.8% diff

EOM


The verbose numbers section.
If you note, the numbers below were used up top

w = MiqWidget.find_by(name: 'report_top_storage_consumers') ; opts = w.group_options.last
bookend("generate", gc: true) { w.send(:content_generator).generate(w, *opts) }
ms bytes objects queries query (ms) rows comments
8,048.9 117,580,084* 9,798,996 1,196 2,507.0 6,768 before.1
5,948.2 96,799,316* 6,407,554 1,196 1,463.2 6,768 before.2
5,994.2 51,616,590* 6,412,285 1,196 1,491.9 6,768 before.3
5,962.8 51,785,052* 6,412,174 1,196 1,570.6 6,768 before.4
1,314.7 46,933,349* 1,091,083 56 169.7 16 after.1
249.0 6,975,855 84,669 56 96.5 16 after.2
265.0 6,975,856 84,669 56 101.7 16 after.3
260.9 6,975,857 84,669 56 105.8 16 after.4

* Memory usage does not reflect 3,599,936 freed objects - before.1
* Memory usage does not reflect 2,200,252 freed objects - before.2
* Memory usage does not reflect 2,204,985 freed objects - before.3
* Memory usage does not reflect 2,204,873 freed objects - before.4
* Memory usage does not reflect 9,060 freed objects - after.1

@miq-bot
Copy link
Member

miq-bot commented Mar 7, 2017

Checked commit kbrock@9dab1c4 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 1 offense detected

app/models/metric/helper.rb

Copy link
Member

@isimluk isimluk left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

Nice improvement 👍

@gtanzillo gtanzillo added this to the Sprint 56 Ending Mar 13, 2017 milestone Mar 8, 2017
@gtanzillo gtanzillo merged commit 15f7ba4 into ManageIQ:master Mar 8, 2017
@kbrock kbrock deleted the widget_1 branch March 8, 2017 15:28
@kbrock
Copy link
Member Author

kbrock commented Mar 8, 2017

@chessbyte @gtanzillo you ok marking this as darga/yes?
The BZ is from 5.4

Should be minimal risk (thought I'm unsure why we have to check every record that it is a Metric record vs just the first - not sure the history there)

@kbrock
Copy link
Member Author

kbrock commented Mar 8, 2017

WARNING: these numbers are not right

This was referenced Mar 10, 2017
simaishi pushed a commit that referenced this pull request Mar 20, 2017
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit 58cbec8d9ef464d1b0adf11afe47339dd11b244c
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Wed Mar 8 08:56:49 2017 -0500

    Merge pull request #14224 from kbrock/widget_1
    
    Speed up widget generation
    (cherry picked from commit 15f7ba471e1a31a482a76168b6a07a19b2cf9917)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1434150

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.

5 participants