These are chat archives for ManageIQ/manageiq/performance

27th
Mar 2017
Jason Frey
@Fryguy
Mar 27 2017 15:26
@NickLaMuro I did a quick peek at why azure regions was so high on the allocations list...it turns out it's actually gettext
More specifically, the first time you call _("some string")
First time you call that you get ~800000 allocations
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:27
heh, yeah, that makes sense
Jason Frey
@Fryguy
Mar 27 2017 15:27
which I guess makes sense if it loads the catalogs
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:27
yup
Jason Frey
@Fryguy
Mar 27 2017 15:27
what I did find strange was that each individual call to _("") is another 1000 allocations
even with the same string
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:27
ouch...
Jason Frey
@Fryguy
Mar 27 2017 15:28
actually ~700...but still...that's a ridiculous amount
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:29
taking a look into that now
Jason Frey
@Fryguy
Mar 27 2017 15:29
irb(main):001:0> GC.stat[:total_allocated_objects]
=> 4890050
irb(main):002:0> _("Australia East"); GC.stat[:total_allocated_objects]
=> 5786523
irb(main):003:0> _("Australia East"); GC.stat[:total_allocated_objects]
=> 5787280
irb(main):004:0> _("Australia East"); GC.stat[:total_allocated_objects]
=> 5788037
irb(main):008:0> 1000.times { _("Australia East") }
=> 1000
irb(main):009:0> GC.stat[:total_allocated_objects]
=> 5793246
Jason Frey
@Fryguy
Mar 27 2017 15:34
Note that "Australia East" specifically, is not in the .po catalogs yet...probably because the catalogs haven't been regenerated
so this could be an edge case where a catalog entry doesn't exist
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:35
@Fryguy hmmm, I am only 7 on first load and 4 after doing the following:
before = GC.stat[:total_allocated_objects]; _("Australia Eastaman"); puts GC.stat[:total_allocated_objects] - before
Jason Frey
@Fryguy
Mar 27 2017 15:35
nope, nevermind, I tried it with "Dialog" which is in the catalog, and same thing
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:35
@Fryguy I think this is a prod/dev thing
I am running in production mode
Jason Frey
@Fryguy
Mar 27 2017 15:36
could be
ah...I run in dev
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:36
tsk tsk
Jason Frey
@Fryguy
Mar 27 2017 15:36
does gettext have prod/dev differences?
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:36
not that I can see, but it is calling a "super" which I can't find yet
oh, probably from "fast_gettext"
Jason Frey
@Fryguy
Mar 27 2017 15:39
oh...hmm...I might be misattributing...
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:39
yeah, looks like fast_gettext has a cache
Jason Frey
@Fryguy
Mar 27 2017 15:40
fun fact, GC.stat itself takes ~350 allocations:
irb(main):009:0> GC.stat[:total_allocated_objects]
=> 8790842
irb(main):010:0> GC.stat[:total_allocated_objects]
=> 8791216
Keenan Brock
@kbrock
Mar 27 2017 15:40
I'm not so sure there
I've had the numbers much smaller
Jason Frey
@Fryguy
Mar 27 2017 15:40
(could be irb as well creating things for output purposes)
Keenan Brock
@kbrock
Mar 27 2017 15:41
>[GC.stat[:total_allocated_objects], GC.stat[:total_allocated_objects], GC.stat[:total_allocated_objects]]
# => [7035181, 7035182, 7035183]
Jason Frey
@Fryguy
Mar 27 2017 15:41
yeah...probably irb: GC.stat[:total_allocated_objects] - GC.stat[:total_allocated_objects] # => -1
yup
Keenan Brock
@kbrock
Mar 27 2017 15:41
that is why I use that over object space - that is a pig
Jason Frey
@Fryguy
Mar 27 2017 15:41
man there are so many moving parts ༼ ༎ຶ ෴ ༎ຶ༽
Keenan Brock
@kbrock
Mar 27 2017 15:41
lol
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:42
$ ruby -e "puts GC.stat[:total_allocated_objects]; puts GC.stat[:total_allocated_objects]"
72377
72383
Jason Frey
@Fryguy
Mar 27 2017 15:42
very interesting
Keenan Brock
@kbrock
Mar 27 2017 15:42
ok, squirel?
Jason Frey
@Fryguy
Mar 27 2017 15:43
yeah, probably...was trying to understand why azure regions file was overloaded, but I noticed it uses gettext, which is 800000 allocations
then stumbled upon this oddity
thanks for helping narrow down! I feel like it still takes ~350 allocations per call, which maybe something we can look at, but it's probably not contributing much to overall memory growth
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:51

@Fryguy So, I have my benchmark script I shared last week, specifically on azure-armrest:

$ bin/benchmark azure-armrest

And it returns a result of about "20 MiB". When swaping out the puts for TOP_REQUIRE.print_sorted_children, on of the biggest contributors to that 20 is rest-client ( 16 MiB )

and inside of that, 10 of that comes from mime/types
that said, this is something that is used by about 4 other gems, so that is a one time cost it seems
Keenan Brock
@kbrock
Mar 27 2017 15:53
I have to say, mime/types frustrates me.
Even including apache, it seems like we only care about a very small number of mime types
yet we have a list of hundreds in memory
and very smart derivation. which could have been done at "compile time" not "run time".
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:57
Well, good luck getting rid of it, because it is a direct dependency of rest-client: https://github.com/rest-client/rest-client/blob/master/rest-client.gemspec#L27
and that is a direct dependency of about 5 other gems in our project
Keenan Brock
@kbrock
Mar 27 2017 15:57
piece of cake, lets just fork them :(
Nick LaMuro
@NickLaMuro
Mar 27 2017 15:57
I am with you on the "it sucks" camp, but that sounds like a tall order for 10 MiB
Jason Frey
@Fryguy
Mar 27 2017 15:57
yes, mime/types is a problem...that's why this was created: https://github.com/discourse/mini_mime
which has already been accepted in the mail gem, but is waiting for rest-client to accept it
rest-client/rest-client#557
mikel/mail#1060
been dying for that to get merged into rest-client :(
Keenan Brock
@kbrock
Mar 27 2017 16:00
That said, at a much higher level the majority of what the mime types gem stores is unneeded information for the 2 giant consumers of the gem in the wild (mail and rest-client) I think full featured "mime types" is unneeded for 99% of the consumers, no need to ship all this complex logic and so on for the general case.
(from that open pr)
Nick LaMuro
@NickLaMuro
Mar 27 2017 16:01
@Fryguy @kbrock If we want, we could right a fake mime-types stub that just redirects to mini-mime, and include that as a dependency
it is a hack, but might be a short term fix
Keenan Brock
@kbrock
Mar 27 2017 16:02
so we'd write mime-types but point to a github repo?
and internally, it is basically 3 files
Nick LaMuro
@NickLaMuro
Mar 27 2017 16:02
or just a local path
Keenan Brock
@kbrock
Mar 27 2017 16:03
heh
there is that
Nick LaMuro
@NickLaMuro
Mar 27 2017 16:03
let me try spiking on that... curious if it will work
Keenan Brock
@kbrock
Mar 27 2017 16:03
yea, path gem is simple
we did that for pending gems for a while
Nick LaMuro
@NickLaMuro
Mar 27 2017 16:04
Not so much the path part, but the overriding part
Nick LaMuro
@NickLaMuro
Mar 27 2017 16:45
@Fryguy @kbrock Well, got it to at least load things with out breaking with about 20 lines of code, but not seeing too much of a difference. Will toss up a PR shortly and you can try it yourself.
Keenan Brock
@kbrock
Mar 27 2017 16:46
thanks
very cool stuff This sounds neat. looking forward to it
Nick LaMuro
@NickLaMuro
Mar 27 2017 16:49
you haven't seen it yet...
Keenan Brock
@kbrock
Mar 27 2017 16:49
^ fixed
Nick LaMuro
@NickLaMuro
Mar 27 2017 16:52
you are like the Grandma after a 5th grade band concert: "Oh wow! That was great my little Beethoven." after the band's rendition of "Hot Cross Buns"
Nick LaMuro
@NickLaMuro
Mar 27 2017 17:02
@kbrock ManageIQ/manageiq#14525
@Fryguy ^
Kinda ugly, and really should tack on the [WIP] label
Jason Frey
@Fryguy
Mar 27 2017 17:04
:100: points for Tangled reference :laughing:
Keenan Brock
@kbrock
Mar 27 2017 17:05
@NickLaMuro can you run numbers for memory for this?
Nick LaMuro
@NickLaMuro
Mar 27 2017 17:05
@Fryguy Oh, that what that was from... I just Google Image searched "mime" and "mini mime"...
Keenan Brock
@kbrock
Mar 27 2017 17:05
Oh an, very nice job Mr. Beethoven
Nick LaMuro
@NickLaMuro
Mar 27 2017 17:05
:facepalm:...
@kbrock so when I did, I wasn't getting very consistent numbers, but when just loading the azure-armrest gem, it only when from ~20 MiB to ~17 MiB
(which has rest-client as a dep)
Keenan Brock
@kbrock
Mar 27 2017 17:10
wish ruby had a decent http client :(
rather than all the options out there (which end up all being included in your project)
anyway
Nick LaMuro
@NickLaMuro
Mar 27 2017 17:22
@kbrock Correction from my earlier number... seems like I am getting around 12 MiB with loading just azure-armrest
so seems to be a bigger savings then I originally measured...
Keenan Brock
@kbrock
Mar 27 2017 17:23
@NickLaMuro yea, I think this will be a big savings. Mr. Mozart
Nick LaMuro
@NickLaMuro
Mar 27 2017 17:24
only about 3% of our overall mem footprint on load
and I don't think the 10MiB increases over time
@kbrock so it is nice, but it will be a journey... one which I am sure you are familiar with Grandma
Keenan Brock
@kbrock
Mar 27 2017 17:26
(I'm really the :wolf: pretending to be :older_woman: ) Mr. Bach
Joe Rafaniello
@jrafanie
Mar 27 2017 17:46
Nice @NickLaMuro
any savings on rails boot is multiplied since ruby isn't CoW friendly (so fork doesn't share that much memory after a 'while')
Keenan Brock
@kbrock
Mar 27 2017 17:47
:wolf: isn't :cow: friendly?
Nick LaMuro
@NickLaMuro
Mar 27 2017 17:56
@kbrock Oh, and I think this answers the question of "oh, but we only use 2-3 mime-types": https://github.com/ManageIQ/manageiq/blob/master/app/models/filesystem.rb#L111-L118
which, looks like we don't... we potentially could be using all of them...
Keenan Brock
@kbrock
Mar 27 2017 17:58
well, I'm glad we load the :pig: into our app then
Eric Hayes
@hayesr
Mar 27 2017 17:58
Is this the performance room or barnyard emoji room?
Joe Rafaniello
@jrafanie
Mar 27 2017 18:01
is there a difference?
Eric Hayes
@hayesr
Mar 27 2017 18:01
:smile:
Nick LaMuro
@NickLaMuro
Mar 27 2017 18:04
:musical_note: "Old McKbrock had a farm..." :musical_note:
Keenan Brock
@kbrock
Mar 27 2017 18:05
Nice song Mr. Handel
Nick LaMuro
@NickLaMuro
Mar 27 2017 18:06
clearly we are playing the "who can last the longest continuing this lame joke" game...
Keenan Brock
@kbrock
Mar 27 2017 18:07
Yea, but I have easy lookups Mr. Brahms
Nick LaMuro
@NickLaMuro
Mar 27 2017 18:08
I am more of a Debussy kinda guy
Keenan Brock
@kbrock
Mar 27 2017 18:09
hey Sri, youtube Debussy
ooh no. she ratted me out
Aaah, "Clair de Lune"
pain when you know a name and can't remember why
and with 13 million views, and no "cats" or "epic fails", this gives me faith in humanity (for a few minutes at least)
@NickLaMuro not sure if @chessbyte has a place where he would like to store a "gem" like this
I know the goal has been to get gems out of our app. not sure if a monkey-patched psudo gem is something he would has a preference
Nick LaMuro
@NickLaMuro
Mar 27 2017 18:16
yeah, totally agree. This probably isn't it's final form, because I am sure this is just a gross way to solve the problem
should have probably tossed the [POC] label on there as well
Oleg Barenboim
@chessbyte
Mar 27 2017 18:27
what is the question??
Nick LaMuro
@NickLaMuro
Mar 27 2017 18:33

@chessbyte basically, "Does Oleg hate how this looks (currently): ManageIQ/manageiq#14525 "

Assumed answer: yes

Roughly 10 MB cheaper to load than mime-types
Keenan Brock
@kbrock
Mar 27 2017 18:41
mime-types is a pig
Oleg Barenboim
@chessbyte
Mar 27 2017 18:47
why is this new gem inside ManageIQ/manageiq repo?
Joe Rafaniello
@jrafanie
Mar 27 2017 18:56
probably because it's just a prototype
Nick LaMuro
@NickLaMuro
Mar 27 2017 19:04
mime-types is a direct dependency of rest-client, so we needed a gem stub to redirect it's calls from that to mini-mime. And yes, as this was a proof of concept to see if doing this would drop the memory usage, I wasn't going to create a new repo just to test itr out
Joe Rafaniello
@jrafanie
Mar 27 2017 19:13
Clearly, he wanted to use the mime pictures in the main repo for the extra traffic
;-)
you can admit it @NickLaMuro, I'm on to you
Keenan Brock
@kbrock
Mar 27 2017 19:15
@chessbyte many gems are pointing to a bloated gem (mime-types) So nick's code monkeypatches that dependency and points it to a new gem.
it is the same kind of problem we had years ago with multiple json gems
and are still having with multiple http client gems
Oleg Barenboim
@chessbyte
Mar 27 2017 19:15
I am +1 conceptually, but defer to @Fryguy for details
Keenan Brock
@kbrock
Mar 27 2017 19:15
thnx
Nick LaMuro
@NickLaMuro
Mar 27 2017 19:22
@jrafanie truthfully, I found those pictures only after I was mostly done with the POC... but if it makes you happy... sure, I am full of myself
Joe Rafaniello
@jrafanie
Mar 27 2017 19:24
haha
Keenan Brock
@kbrock
Mar 27 2017 20:26
I wonder if there is a way to improve our MiqQueue insert to reduce the number of queries
  1. convert the fetch / insert -> insert (with ruby conditional?)
  2. remove those darn begin ends

Back on perf_capture_timer...

ms queries query (ms) rows comments
22,316.0 12,921 7,277.4 116 pct: openshift-1
22,284.7 .Metric::Capture.perf_capture_timer
2.0 1 0.8 ..Metric::Capture.perf_capture_health_check
376.2 ..Metric::Targets.capture_targets
81.4 2 77.6 1 ...Metric::Targets.capture_infra_targets
294.7 7 195.4 68 ...Metric::Targets.capture_container_targets
3.3 ..Metric::Capture.filter_perf_capture_now
21,880.7 12,907 6,986.3 47 ..Metric::Capture.queue_captures
21.8 4 17.3 ..MiqTask.delete_older

all the queries (12,907/12,921) are inserting into miq_queue

we could half that if inserting into the queue was a single query
also, determining the zone (on every insert sometimes) can be expensive, but this case I've basically removed that entirely
Jason Frey
@Fryguy
Mar 27 2017 20:33
is this a put_unless_exists?
Keenan Brock
@kbrock
Mar 27 2017 21:12
yes
since I run this multiple times, the numbers get botched. too hard to make multiple runs perform exactly the same
but
ms queries query (ms) rows comments
23,912.9 12,964 8,368.7 119 openshift#-1
21,625.8 (12,869) 6,468.4 9 ..Metric::Capture.queue_captures
3,458.5 3,215 3,117.4 ...SELECT "miq_queue".*
704.2 3,215 704.2 ...BEGIN
1,544.7 3,215 1,544.7 ...INSERT INTO "miq_queue"
754.4 3,215 754.4 ...COMMIT
bring back the miq_queue records brings back many columns
Keenan Brock
@kbrock
Mar 27 2017 21:27
@Fryguy cool ==> http://stackoverflow.com/questions/1109061/insert-on-duplicate-update-in-postgresql
not sure the implications here, but looks promising