These are chat archives for ManageIQ/manageiq/performance

14th
Jan 2016
Keenan Brock
@kbrock
Jan 14 2016 04:00
@matthewd turkey - rand me deep into a rat hole. Have the answer for the inverse_of but... cost a whole day ;)
Matthew Draper
@matthewd
Jan 14 2016 04:01
:mouse:
is that what you were looking for?
looks like the default scope costs us a bit
and the whole vms, vm_and_templates, vm_or_templates - and the Vm vs VmOrTemplate
now I'm going to run those queries again and see what is blowing up. I think my new invese of will be better educated
Matthew Draper
@matthewd
Jan 14 2016 04:03
Ah, yes, this looks insightful indeed
Keenan Brock
@kbrock
Jan 14 2016 04:03
fun - so the script should detect unnecessary :instance_of
:)
(that was extra credit - right?)
Matthew Draper
@matthewd
Jan 14 2016 04:04
Yes, actually. Compared to us potentially going on an inverse_of-plastering expedition, being able to understand exactly where/why it's needed is a huge improvement
Keenan Brock
@kbrock
Jan 14 2016 04:04
looking at how our classes have changed, I can see how we would have broken automatic_instance_of more and more. but it will look like the framework's fault
or ruby's fault
Matthew Draper
@matthewd
Jan 14 2016 04:05
So far it looks like the biggest thing is the subclassing
Keenan Brock
@kbrock
Jan 14 2016 04:05
we are not consistent in our use of 'vms' and 'VmOrTemplate'
Matthew Draper
@matthewd
Jan 14 2016 04:05
(outside of the obvious/known things you can actually see right on the corresponding has_many call)
Should there be instances of "(there is a default scope)"?
Keenan Brock
@kbrock
Jan 14 2016 04:06
huh
I used to see them
e.g. Vm has a default scope
so nothing will inverse to it
ugh
so many nuances
Matthew Draper
@matthewd
Jan 14 2016 04:07
I think I find that surprising… maybe.
Keenan Brock
@kbrock
Jan 14 2016 04:07
and this detection script will break for other versions of rails
hmm
the association had a 'scope' on it
Matthew Draper
@matthewd
Jan 14 2016 04:08
Yeah, 'cos it's basically reimplementing the auto_inverse_of logic in pieces, so depends on said logic not changing
Keenan Brock
@kbrock
Jan 14 2016 04:08
yes
I was getting confused if I should be looking at the delegated reflection or the reflection
but I think it is all the same, as the :throughs are rejected anyway
Matthew Draper
@matthewd
Jan 14 2016 04:09
Yeah, pretty sure a through can't have an inverse
Keenan Brock
@kbrock
Jan 14 2016 04:10
huh, I had a number of associations to vm having a scope
now not so much
interestingly valid_inverse_reflection? calls can_find_inverse_of_automatically? - but I think it is a dup
automatic_inverse_of already called it

ref

def automatic_inverse_of
  if can_find_inverse_of_automatically?
    if valid_inverse_reflection?
    end
  end
end

def valid_inverse_reflection?
  ... && can_find_inverse_of_automatically?
end

although - I may have misread

Matthew Draper
@matthewd
Jan 14 2016 04:15
Yeah.. not a dup
Keenan Brock
@kbrock
Jan 14 2016 04:15
aah - self and reflection
Matthew Draper
@matthewd
Jan 14 2016 04:15
That's verifying that both sides are auto-inverse capable
Keenan Brock
@kbrock
Jan 14 2016 04:16
hmm, I'll add that to the method then
ugh
Matthew Draper
@matthewd
Jan 14 2016 04:16
It sounds to me like we could improve things with the subclass mismatch
Keenan Brock
@kbrock
Jan 14 2016 04:17
"we" = miq?
patch rails or add :instance_of?
Matthew Draper
@matthewd
Jan 14 2016 04:17
rails
Keenan Brock
@kbrock
Jan 14 2016 04:23
ooh. I did 1 case statement, what I needed to do was reimplement the rails methods - but instead of returning true / false (string), return that plus a reason.
do I want to improve this?
or is it good enough?
Matthew Draper
@matthewd
Jan 14 2016 04:24
I think it's good enough
Keenan Brock
@kbrock
Jan 14 2016 04:24
would be cool to implement a module
that took boolean logic
and converted to boolean logic, which statement failed
Matthew Draper
@matthewd
Jan 14 2016 04:25
Unless/until we run into a whole block of associations we're trying to inversify and discover it's outright lying or something, I think it gives us enough info to decide whether we need to manually do a thing
Joe Rafaniello
@jrafanie
Jan 14 2016 04:25
:moon: :sleeping:
Keenan Brock
@kbrock
Jan 14 2016 04:25
lol
:sunny: :sleeping: for @matthewd
@jrafanie you are partially guilty for this one
Joe Rafaniello
@jrafanie
Jan 14 2016 04:25
Sleep on it
Keenan Brock
@kbrock
Jan 14 2016 04:26
we've found a slew of missing :instance_of :inverse_of - TONS
aah - @matthewd
so in rails, I could have sworn if you did Vm.first 10 times, you'd get only 1 query to the db. but only from a controller
Joe Rafaniello
@jrafanie
Jan 14 2016 04:26
Yay!
Keenan Brock
@kbrock
Jan 14 2016 04:26
am I smoking crack?
Matthew Draper
@matthewd
Jan 14 2016 04:27
One real query, plus 9 cached queries
Keenan Brock
@kbrock
Jan 14 2016 04:27
but if I do that from rails console - that doesn't hold true
Joe Rafaniello
@jrafanie
Jan 14 2016 04:27
Missing inverse_of or instance_of?
Keenan Brock
@kbrock
Jan 14 2016 04:27
is there a way for me to wrap my stuff in a block so I can exhibit that behavior
sorry - :inverse_of - thanks
Joe Rafaniello
@jrafanie
Jan 14 2016 04:28
Ok
Keenan Brock
@kbrock
Jan 14 2016 04:29
either of you - offhand, do you know of a tool like rpm that I can use for profiling?
Matthew Draper
@matthewd
Jan 14 2016 04:29
ActiveRecord::QueryCache.cache do .. end maybe?
Keenan Brock
@kbrock
Jan 14 2016 04:29
oooh - that looks promising
heh. put that around every miqqueue pop ;)
Joe Rafaniello
@jrafanie
Jan 14 2016 04:30
Hehe
Matthew Draper
@matthewd
Jan 14 2016 04:30
That's actually a perfectly reasonable idea, IMO
Joe Rafaniello
@jrafanie
Jan 14 2016 04:31
Ok. Well I'll send this again since I'm out. :moon:
Keenan Brock
@kbrock
Jan 14 2016 04:32
:peach: :outbox_tray:
(peace out)
(peach out)
Matthew Draper
@matthewd
Jan 14 2016 04:32
Ah, looks like cache ends up on AR::Base
Keenan Brock
@kbrock
Jan 14 2016 04:32
ooh
Matthew Draper
@matthewd
Jan 14 2016 04:32
But is also maybe not doing the thing? QueryCache#call seems to do Other Stuff.
Nope, sounds like it should work
Keenan Brock
@kbrock
Jan 14 2016 04:35
ok
rails idea
vms - no inverse reflection for tenant
aah
the output is old
fixing gist
Matthew Draper
@matthewd
Jan 14 2016 04:38
Is that related to the fact that Vm is claiming reflections that actually live on VmOrTemplate?
Keenan Brock
@kbrock
Jan 14 2016 04:38
that is good enough
Matthew Draper
@matthewd
Jan 14 2016 04:40
vm_or_templates :neutral_face:
Keenan Brock
@kbrock
Jan 14 2016 04:40
vs vm_and_templates :trollface:
lol
Matthew Draper
@matthewd
Jan 14 2016 04:40
Yeah.. looks like tenant's association is spelled wrong
Keenan Brock
@kbrock
Jan 14 2016 04:40
lol
yay
Matthew Draper
@matthewd
Jan 14 2016 04:40
wtf
VmOrTemplate tenant NO
Tenant vm_or_templates YES
Isn't that not supposed to happen? :confused:
Keenan Brock
@kbrock
Jan 14 2016 04:41
perfect
ok
so the parent class has the association
the child class doesn't
it doesn't find it
that was the case that I wanted to share
that could be an error with my logic
Matthew Draper
@matthewd
Jan 14 2016 04:42
The final YES/NO should probably be looking at .inverse_of
Keenan Brock
@kbrock
Jan 14 2016 04:43
good one
Matthew Draper
@matthewd
Jan 14 2016 04:43
So worst case we get YES + a reason it should be no, or NO + automatic_inverse_of, instead of an outright lie about what's happening
Keenan Brock
@kbrock
Jan 14 2016 04:43
also s/automatic_inverse_of/automatic/ ?
too many words
Matthew Draper
@matthewd
Jan 14 2016 04:44
:+1:
Keenan Brock
@kbrock
Jan 14 2016 04:45
is there a public for _reflect_on_association ?
Matthew Draper
@matthewd
Jan 14 2016 04:45
Without the underscore, maybe?
Keenan Brock
@kbrock
Jan 14 2016 04:46
thanks
should I use actual calls, or public calls?
Matthew Draper
@matthewd
Jan 14 2016 04:46
In that case, go with public for ease
Only difference is that the public one is symbol/string agnostic, for back compat
At least I thought so.. experimentally they seem identical (and both public?)
Keenan Brock
@kbrock
Jan 14 2016 04:47
both public
think _ is marked/commented @api private
Matthew Draper
@matthewd
Jan 14 2016 04:47
Ah, right
In that case, it doesn't really matter what you use… your whole logic depends on the exact form of the current code anyway ¯\_(ツ)_/¯
Keenan Brock
@kbrock
Jan 14 2016 04:52
yea
I'm rewriting
to match code
just returning 2 values
have you worked much with transforming code
e.g.
def abc(x)
  if x
    'T'
  else
    'F'
  end
end

# ==>

def abc(x)
  if x
    ['T', 'x']
  else
    ['F', '!x']
  end
end
Matthew Draper
@matthewd
Jan 14 2016 04:54
Seems tricky to do automagically
Keenan Brock
@kbrock
Jan 14 2016 04:54
I remember with aspect orient programming, there was a whole push to have java rewrite the java bytecode onthe fly and giving people erb like templates to do so
crystal has macros that will do that too
s/erb/jsp/
Matthew Draper
@matthewd
Jan 14 2016 04:55
I'd create a class LoggingBool < Struct.new(:value, :reason); def & ..; def | .., then manually change &&/|| to &/|, I think
Keenan Brock
@kbrock
Jan 14 2016 04:56
I just wrote
def reason_inverse_name(src, ref)
  value, reason = reason_automatic_inverse_of(src, ref)
  if ref.options[:inverse_of]
    if ref.options[:inverse_of] == value
      [ref.options[:inverse_of], "UNNEEDED: options[:inverse_of]"]
    else
      [ref.options[:inverse_of], "options[:inverse_of]"]
    end
  else
    [value, reason]
  end
end
much better than the case statement I had before
and close to the code:
        def inverse_name
          options.fetch(:inverse_of) do
            if @automatic_inverse_of == false
              nil
            else
              @automatic_inverse_of ||= automatic_inverse_of
            end
          end
        end
of course, I'm being cute with the "rubocop" warning - it could be easier
Matthew Draper
@matthewd
Jan 14 2016 04:57
OTOH, you could skip that part entirely
Keenan Brock
@kbrock
Jan 14 2016 04:58
well, that is the gist for the 4 layers deep
Matthew Draper
@matthewd
Jan 14 2016 04:58
If you call the real inverse_of and automatic_inverse_of, you can answer [UNNEEDED, auto, options[:inverse_of], no] without any effort
Keenan Brock
@kbrock
Jan 14 2016 04:58
once I have to rewrite the can_find_inverse_of_automatically for inverse and regular, I realize I need to do more
Matthew Draper
@matthewd
Jan 14 2016 04:59
Then you only need the reason if automatic_inverse_of returned no
So no tuples… it just becomes a method that returns the reason it couldn't find an inverse
Keenan Brock
@kbrock
Jan 14 2016 04:59
but don't you wonder if the auto value coming back matches the value you hardcoded?
if it does not, then it would say 'unneeded' when in fact it is
Matthew Draper
@matthewd
Jan 14 2016 05:00
Call the real automatic_inverse_of directly
Keenan Brock
@kbrock
Jan 14 2016 05:02
def reason_inverse_name(src, ref)
  if ref.options[:inverse_of]
    if ref.options[:inverse_of] == ref.send(:automatic_inverse_of)
      "UNNEEDED: options[:inverse_of]"
    else
      "options[:inverse_of]"
    end
  else
    reason_automatic_inverse_of(src, ref)
  end
end
Matthew Draper
@matthewd
Jan 14 2016 05:05
Where a nil there means 'auto'? Yeah, that's what I had in mind.
Keenan Brock
@kbrock
Jan 14 2016 05:06
options[:inverse_of] can be false
yea, nil means auto
Matthew Draper
@matthewd
Jan 14 2016 05:06
Oh, true, it should check for an explicit false
I guess the real condition is ref.inverse_of — if that's set, we're checking whether it came from an explicit option, and whether the option was required; if it's nil, then we're looking for a reason (which might be "option was false")
Keenan Brock
@kbrock
Jan 14 2016 05:12
yea - but it is nice, I don't care what the value is. just the reason (for true or false)
Keenan Brock
@kbrock
Jan 14 2016 13:28
@matthewd you looked much at https://github.com/rails/rails-perftest ?
Jason Frey
@Fryguy
Jan 14 2016 15:52
I think it would be really cool if after all this effort we could turn it into a spec
Like I would love if someone adds a new relationship and it needs a manual inverse_of, then the spec could blow up
@kbrock Regardless of the work done here...have you verified for that particular PR which of those lines are necessary/unnecessary ?
If you can get it down to the necessary lines, then we can merge that, and then worry about checking all other models in the system separately
because that PR solves a specific, known, problem
Keenan Brock
@kbrock
Jan 14 2016 16:16
looks like alex said it didn't show him any benifits :(
Alex Krzos
@akrzos
Jan 14 2016 17:23
@kbrock I still think we need to profile whats occuring on 5.5
Keenan Brock
@kbrock
Jan 14 2016 17:23
yea
ugh - last version - I promise: inverse_of
(has suggestions)
@jrafanie @Fryguy I'm back again in this :mouse: hole - is there a good way to profile a large method and unsersatnd what is happening in there - akin to rpm?
Jason Frey
@Fryguy
Jan 14 2016 17:25
ruby-prof?
Keenan Brock
@kbrock
Jan 14 2016 17:25
you can't run that on production - right?
Matthew Draper
@matthewd
Jan 14 2016 17:25
rack-mini-profiler ?
Jason Frey
@Fryguy
Jan 14 2016 17:25
why not
Keenan Brock
@kbrock
Jan 14 2016 17:25
the worker job takes 6(?) minutes to run
Jason Frey
@Fryguy
Jan 14 2016 17:25
I don't see why that would be a problem
Matthew Draper
@matthewd
Jan 14 2016 17:25
(with called methods tagged for individual timing)
Keenan Brock
@kbrock
Jan 14 2016 17:26
@matthewd have you found out how to rack-mini-profiler from a daemon (not http request?)
Jason Frey
@Fryguy
Jan 14 2016 17:26
@kbrock Do you have a link for rpm? I don't know that tool and googling for it is obviously bringing me to "red hat package manager" ;)
Keenan Brock
@kbrock
Jan 14 2016 17:26
new relic rpm
skylight
hmm - that one is vague too - lol
Jason Frey
@Fryguy
Jan 14 2016 17:26
we can't use skylight
Keenan Brock
@kbrock
Jan 14 2016 17:27
in production
Jason Frey
@Fryguy
Jan 14 2016 17:27
we don't run a website
Keenan Brock
@kbrock
Jan 14 2016 17:27
don't be so quick
Jason Frey
@Fryguy
Jan 14 2016 17:27
?
Keenan Brock
@kbrock
Jan 14 2016 17:27
those tools work with background jobs
Matthew Draper
@matthewd
Jan 14 2016 17:27
@kbrock I just stuck a call to "do the thing I care about" in a bare rack app, then went to the URL
Jason Frey
@Fryguy
Jan 14 2016 17:27
their pricing model doesn't fit ours
Keenan Brock
@kbrock
Jan 14 2016 17:27
I don't want to ship it
I just want to profile my own code
ok
Jason Frey
@Fryguy
Jan 14 2016 17:27
when you say skylight, do you mean https://www.skylight.io/
Keenan Brock
@kbrock
Jan 14 2016 17:28
yes
Jason Frey
@Fryguy
Jan 14 2016 17:28
If there is a way to use skylight in dev, that would be cool, but my research has shown that not to be possible
Keenan Brock
@kbrock
Jan 14 2016 17:29
yea - new relic does that
but it is in memory / ... difficult for us to use
@matthewd nice
Matthew Draper
@matthewd
Jan 14 2016 17:34
Skylight might Just Work under the free plan to measure locally
Keenan Brock
@kbrock
Jan 14 2016 17:34
hmm
Jason Frey
@Fryguy
Jan 14 2016 17:34
possibly...we'd have to have a "production" like instance to mearsure real world data
Matthew Draper
@matthewd
Jan 14 2016 17:39
Yeah, I think we'd be looking just at the "deeper" analysis, rather than the overall sampled response performance type stuff
Jason Frey
@Fryguy
Jan 14 2016 17:39
ah gotcha
Yeah, like the breakdowns they do for single request/response could be really valuable
Keenan Brock
@kbrock
Jan 14 2016 18:27
UUUUGGGGGHHHH - I'm now showing 0 changes for these queries
wtf
Jason Frey
@Fryguy
Jan 14 2016 18:28
o_O
Keenan Brock
@kbrock
Jan 14 2016 18:28
my database is showing 0 differences with these queries
Jason Frey
@Fryguy
Jan 14 2016 18:28
when you are querying perf_capture_timer are you clearing out all the "last checked on " values?
Keenan Brock
@kbrock
Jan 14 2016 18:28
it runs 486 queries
Jason Frey
@Fryguy
Jan 14 2016 18:28
there are a bunch of up front eliminations for things we already recently just captured
Keenan Brock
@kbrock
Jan 14 2016 18:28
hmm
Jason Frey
@Fryguy
Jan 14 2016 18:29
you need to have a clean base every time for testing, otherwise it will be different every time
(if you are doing it via a spec, it's always clean, which might be a preferable way to test)
Keenan Brock
@kbrock
Jan 14 2016 18:30
but the problems don't show up in a clean base
they show up when there is data in the table
I'm running these with rollbacks
so the data isn't changing
Jason Frey
@Fryguy
Jan 14 2016 18:30
ok
yeah, that's good
"well, then, I have no idea" :tea:
Keenan Brock
@kbrock
Jan 14 2016 18:30
lol
I'm fighting not reinventing a monitoring tool that will tell me what is slow in a block of code
Jason Frey
@Fryguy
Jan 14 2016 18:31
(one of you imgur masters needs to get me a gif of Keanu Reeves on SNL celebrity jeopardy, saying that line @chrisarcand @blomquisg?)
Keenan Brock
@kbrock
Jan 14 2016 18:31
lol
Greg Blomquist
@blomquisg
Jan 14 2016 19:36
haha, and tweeted
that Alex Trebek face at the end, too
Joe Rafaniello
@jrafanie
Jan 14 2016 19:52
oh man, @chrisarcand that's a special type of skill
Chris Arcand
@chrisarcand
Jan 14 2016 19:52
¯\_(ツ)_/¯First gif I’ve ever made ;) That clearly means it doesn’t require too much of that special skill
Daniel Berger
@djberg96
Jan 14 2016 19:53
I think I've made, like, two ever
and they weren't as nice as Chris'
Chris Arcand
@chrisarcand
Jan 14 2016 19:55
Well the app I used to make it wasn’t running Rails 1.2.6 0.9.4 either :stuck_out_tongue_winking_eye: