These are chat archives for ManageIQ/manageiq/performance

3rd
Oct 2016
Keenan Brock
@kbrock
Oct 03 2016 14:19
@imtayadeway virtual attributes are getting closer to just working: #11631
but this may be going too far...
Chris Arcand
@chrisarcand
Oct 03 2016 14:23
That was my initial thought ;)
Keenan Brock
@kbrock
Oct 03 2016 14:24
I am tempted to add flat_map onto relations so they just do the right thing
we jump through such hoops to go from one class to another
@chrisarcand is there a reason to avoid :through in relations - for caching reasons?
if you use a :through, and the code wants to use the intermediary, it can be worse
Chris Arcand
@chrisarcand
Oct 03 2016 14:25
I’m not sure if having an actual first class model with a :through relation is any slower than a normal HABTM with just a join table.
Keenan Brock
@kbrock
Oct 03 2016 14:26
yea, but if you also want the model in the middle
then you end up loading the target class 2 times
ok
thanks
The virtual attributes bit stems from ManageIQ/manageiq#11630 -
we have a ton of code to calculate counts
and our current solution is to download TONS of records and generate queries with pretty bad IN clauses
Chris Arcand
@chrisarcand
Oct 03 2016 14:44
I’m not understanding the purpose of virtual_delegates, help? From it’s beginning it doesn’t look like anything more than a different sugar than the code it replaces (virtual_column :blah, :uses => :foo) ManageIQ/manageiq@443eb85
Keenan Brock
@kbrock
Oct 03 2016 14:45
@chrisarcand the difference being that we can embed that attribute into sql
so instead of forcing us to fetch all records and bring into ruby
to sort or filter
we can do it in the database
Chris Arcand
@chrisarcand
Oct 03 2016 14:48
What’s the purpose in :uses at this point, then? Should it be deprecated…?
Nick LaMuro
@NickLaMuro
Oct 03 2016 14:52
@chrisarcand the main difference between virtual_delegates and virtual_attributes is it is a virtual_attribute that also defines the arel and ruby method for you.
Keenan Brock
@kbrock
Oct 03 2016 14:52
@chrisarcand yes, :uses should be deprecated, but it was used by 1 case and I didn't want to fight that PR battle when converting it across to using delegates
the long term plan for delegates is to:
  1. define all the places that are just delegates
  2. fix the code so they don't use delegates
easier in my mind to just label something as a delegate - no PR back an forth. Then when we get rid of delegates, it will be easier to speak to tangible code rather than abstract concepts.
because on topics like these it is easy to get into essoteric conversations

@Fryguy do you have a major opinion on this single line change?
https://github.com/ManageIQ/manageiq/pull/11003/files#diff-493a89425411941fae33b3433989d031L337

It prevents us from downloading the whole tree when we are trying to filter.

Chris Arcand
@chrisarcand
Oct 03 2016 14:56
I don’t see what you mean. “just delegates”, you mean straight Ruby delegates using :uses? “Fix code so they don’t use delegates”, you mean “use virtual_delegate?"
Keenan Brock
@kbrock
Oct 03 2016 14:56
change reports so they access the column they actually want
Nick LaMuro
@NickLaMuro
Oct 03 2016 14:56

So for example for this line: https://github.com/ManageIQ/manageiq/blob/ffd25b04/app/models/vm_or_template.rb#L179

It would not only define a virtual_attribute (same thing as a virtual_column), but also the delegate method to be used by ruby, and arel to access it via SQL.

Chris Arcand
@chrisarcand
Oct 03 2016 14:57
right, right.
Keenan Brock
@kbrock
Oct 03 2016 14:57
I guess I'm suggesting to anti-demeterize the reports
either that or write a query optimizer to know what is being called underneath and add to the join clauses.
which if we have declared a object's friends, then that will be easier.
Want to hold off on that conversation
just like with the select PR, we "can" do it, but should we ;)
I want to remove the aggregation mixin. it brings 3 queries and all the ids in the middle to do something that sql was tuned to do
Keenan Brock
@kbrock
Oct 03 2016 17:27

So find_id_by_filtered has logic to fetch a record, then rbac fetch the record
and have a distinction between not found and not authorized

  1. performance - would like 1 query
  2. security, seems better to just handle both and say "not available"

thoughts? [ref]

this is used on pretty much every show page
insult to unjury - it runs sort_by on the query for no reason.