These are chat archives for ManageIQ/manageiq/performance

17th
Jan 2018
Nick LaMuro
@NickLaMuro
Jan 17 2018 04:08

So, this is pretty rough and blobby at the moment, but this is what I have been working on most of today, and a bit of yesterday:

NickLaMuro/miq_tools@a078165

Basically, I can attach to a running ruby process using gdb, and use this script to monitor each malloc, calloc, etc that is made, and determine where in the ruby and c-stack they are being called from
here is an example call from the script we have been testing with that is looping and calling require 'linux_admin' (though, it was triggered by doing a ruby_eval "MiqSystem.num_cpus" in gdb... but same diff):
Screen Shot 2018-01-16 at 9.47.32 PM.png
That specific malloc line happens 6 times in the require call, and the ruby line doesn't change either. Not terribly surprising, but I guess I was expecting to see something in there besides ActiveSupport, and I didn't.
Nick LaMuro
@NickLaMuro
Jan 17 2018 04:13
the rb_expand_load_path though seems suspect (based on a previous discussion), and I will have to dive into the source tomorrow to see what is up, but that will have to wait until tomorrow
night
Jason Frey
@Fryguy
Jan 17 2018 16:18
@NickLaMuro total aside...I was reading this article just now about Rust, and coincindentally the author talks about using Rust to create Ruby profiler that extracts stack traces with only access to the PID and memory maps... https://jvns.ca/blog/2018/01/13/rust-in-2018--way-easier-to-use/
(it doesn't go deeper than that...just thought it was an interesting coincidence)
Keenan Brock
@kbrock
Jan 17 2018 16:18
nice - that is basically why yehuda is touting rust - for https://www.skylight.io/
Nick LaMuro
@NickLaMuro
Jan 17 2018 16:24
@Fryguy no, she was the one that mentioned on twitter that she was going on sabbatical for writing a ruby profiler
also, I had this old article of her's up as well: https://jvns.ca/blog/2016/06/12/a-weird-system-call-process-vm-readv/
which mentions this ariticle which I got a lot of the above for
but yes, I assume it was all co-winky-dink
NickLaMuro @NickLaMuro has a chrome tab problem
Jason Frey
@Fryguy
Jan 17 2018 17:04
@NickLaMuro oh yeah?
rails.bad 2018-01-17 12-04-44.png
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:06
but do you have multiple windows of chrome open?
also, shameless self plug for my S***ty javascript: gist
Jason Frey
@Fryguy
Jan 17 2018 17:10
haha I had one of those pop-up gmail windows open..does that count?
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:11
well, with that, I have 6 windows open, and 123 tabs open
Jason Frey
@Fryguy
Jan 17 2018 17:11
༼ ༎ຶ ෴ ༎ຶ༽ you win
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:11
I have been close to 300 at one point :cry:
the reason I close is mostly because of "hey, I am really using a lot of swap huh... probably should fix that..."
Keenan Brock
@kbrock
Jan 17 2018 17:17
I sometimes bookmark all tabs (put a date in it -knowing I'll never come back)
best way of declaring tab bankrupcy with fooling myself that I'm not doing that
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:18
a lot of it ends up being documentation, which I am trying to push down into my terminal when possible
Keenan Brock
@kbrock
Jan 17 2018 17:18
@NickLaMuro so I'm looking at an appliance that is reusing PIDs 3times / day
Any suggestions for removing false pid dups?
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:18
(using ri for ruby documentation, and recently info for gdb docs (or in my case https://github.com/vim-scripts/info.vim ))
Keenan Brock
@kbrock
Jan 17 2018 17:19
I was thinkng about reading a "exiting worker" and then removing the entry from the hash of hash[PID] => {} (in my logs)
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:19
@kbrock with a lot of my scripts, the regexp usually includes a --worker-type pass in
Jason Frey
@Fryguy
Jan 17 2018 17:19
really useful for tag bankruptcy is the OneTab extension
Keenan Brock
@kbrock
Jan 17 2018 17:20
k - when reading the PIDs from evm.log, not totally known until a little bit
hmm - will look into that. thanks
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:21
we use a lot of sys calls for general things in our app, which doesn't help the problem
NickLaMuro @NickLaMuro is a big stickler for avoiding using syscalls in applications, mind you
Jason Frey
@Fryguy
Jan 17 2018 17:43
@NickLaMuro did you try your linux_admin require patch on an appliance?
there is some caveats with the above, and hopefully I can get some more needed info in a bit
Jason Frey
@Fryguy
Jan 17 2018 17:51
talking with @tenderlove separately, and he hadn't heard of require leaking elsewhere
You could try using malloc stack logging on MacOS. It logs every malloc / free and the stack for that malloc / free.
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:53
now you tell me...
Jason Frey
@Fryguy
Jan 17 2018 17:53
¯\_(ツ)_/¯
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:54

talking with @tenderlove separately, and he hadn't heard of require leaking elsewhere

This doesn't surprise me though, since we did see that a vanilla rails app didn't seem to leak with require (ref: LJ's tests)

Jason Frey
@Fryguy
Jan 17 2018 17:54
yeah, I was hoping against hope
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:54
appreciate you checking though, thanks!
Jason Frey
@Fryguy
Jan 17 2018 17:55

if you set the MallocStackLogging environment variable, you can use the malloc_history commandline tool.

I would either 1) wait until memory grows really huge, then run malloc_history -callTree and find the offender, or 2) take a snapshot before, then run for a bit, and take a second snapshot and compare

Nick LaMuro
@NickLaMuro
Jan 17 2018 17:56
heh, this is pretty much what I ended up doing with gdb
Jason Frey
@Fryguy
Jan 17 2018 17:57
yeah it sounded similar
Nick LaMuro
@NickLaMuro
Jan 17 2018 17:57
except I was taking memdumps based on the smaps location of the heap, after forcing some leaks using
(gdb) p rb_eval_string("MiqSystem.num_cpus")
Joe Rafaniello
@jrafanie
Jan 17 2018 18:27
@NickLaMuro so, sampling in osx shows similar behavior for the "require empty.rb" test in a loop(I started sampling after the process loaded rails (reached 145 or so MB)
blob
Jason Frey
@Fryguy
Jan 17 2018 18:30
I'm not sure what I am looking at there (or what I am supposed to see)
Nick LaMuro
@NickLaMuro
Jan 17 2018 18:30

@Fryguy the call stack is the inverse of what I was showing here in my screenshot:

https://gitter.im/ManageIQ/manageiq/performance?at=5a5ecc316117191e6175158b

Jason Frey
@Fryguy
Jan 17 2018 18:31
right, but I don't know what either of those mean
Joe Rafaniello
@jrafanie
Jan 17 2018 18:34
It's interesting, I don't see rb_feature_p in the vanilla rails top samples
0.441s above :point_up:
Jason Frey
@Fryguy
Jan 17 2018 18:35
ok?
Nick LaMuro
@NickLaMuro
Jan 17 2018 18:35

well, mine specifically is one invocation of malloc and the stacktrace at the point of the malloc call, and that shows that it is mostly happening in c. @jrafanie's screen shot at least shows that he is getting the same callstack as well.

What that means, to me at least, is that it is somehow how the strings and our environment is setup that is messing with the ruby internals to cause the leak. At least that is the conclusion that I am at currently

Joe Rafaniello
@jrafanie
Jan 17 2018 18:35
note rb_find_file_safe in the "vanilla rails"
Nick LaMuro
@NickLaMuro
Jan 17 2018 18:35
need to do more digging
Joe Rafaniello
@jrafanie
Jan 17 2018 18:35
blob
that's vanilla rails :point_up:
Jason Frey
@Fryguy
Jan 17 2018 18:35
and that shows that it is mostly happening in c
mallocs always happen in C, so that's somewhat obvious?
Nick LaMuro
@NickLaMuro
Jan 17 2018 18:36
what I meant was everything from the require on down
there isn't some magical c ext getting in the way
Jason Frey
@Fryguy
Jan 17 2018 18:36
ahhh...ok
Joe Rafaniello
@jrafanie
Jan 17 2018 18:36
So, @Fryguy I'm thinking rb_require_internal is going the route of rb_feature_p in our case when it spends most of it's time in rb_find_file_safe in vanilla rails
Jason Frey
@Fryguy
Jan 17 2018 18:36
right, we are looking at the raw require calling malloc directly
Joe Rafaniello
@jrafanie
Jan 17 2018 18:37
I'm now going to figure out what those things do
Jason Frey
@Fryguy
Jan 17 2018 18:37
rb_find_file_safe is not rails, that's Ruby
I mean, Rails might be calling it, but it's implemented in Ruby
Joe Rafaniello
@jrafanie
Jan 17 2018 18:37
yes, of course
Jason Frey
@Fryguy
Jan 17 2018 18:38

spends most of it's time

but why does time matter?

Nick LaMuro
@NickLaMuro
Jan 17 2018 18:38
@Fryguy I think what Joe and I are getting at is that something in our environment (Pathnames versus strings for things, etc.) is causing require to leak for some reason
or rather, that is the theory for me at the moment (not being a c or ruby internals expert currently)
Joe Rafaniello
@jrafanie
Jan 17 2018 18:38
vanilla rails is blazing fast... ours is slow... If I can sample where it's spending it's time, that might lead to what it doing differently
Jason Frey
@Fryguy
Jan 17 2018 18:39
ok, yes, I agree it's clearly in our environment, but profiling based on time when it's a leak seems like opposite goals
interesting theory
Joe Rafaniello
@jrafanie
Jan 17 2018 18:39
If there was a way to sample by allocations that was easy, I'd be doing that
Nick LaMuro
@NickLaMuro
Jan 17 2018 18:39
@jrafanie that is what I was doing ;)
Joe Rafaniello
@jrafanie
Jan 17 2018 18:39
exactly
Jason Frey
@Fryguy
Jan 17 2018 18:39
if you can track mallocs and frees, then you can see which mallocs are not being freed
and then stacktrace those mallocs back to where the source is
i.e. the tool from @tenderlove :point_up: January 17, 2018 12:51 PM
and :point_up: January 17, 2018 12:55 PM
Joe Rafaniello
@jrafanie
Jan 17 2018 18:41
yeah, I didn't want to do this step quite yet: Guard Malloc is a special version of the malloc library that replaces the standard library during debugging.
Jason Frey
@Fryguy
Jan 17 2018 18:41
seems like it would lead to a fast answer though
Joe Rafaniello
@jrafanie
Jan 17 2018 18:42
and crater my laptop
:wink:
Jason Frey
@Fryguy
Jan 17 2018 18:42
it would only be for that process...would be surprised it would crater it
it might crater it ;)
you're due for a new one right?
Nick LaMuro
@NickLaMuro
Jan 17 2018 18:42
lol
Jason Frey
@Fryguy
Jan 17 2018 18:48
I'm playing with Leaks in Instruments tool on the Mac
never used it before so let's see what happens :)
Nick LaMuro
@NickLaMuro
Jan 17 2018 18:49
bye bye @Fryguy 's laptop :wave:
Jason Frey
@Fryguy
Jan 17 2018 18:49
heh...I literally have the same stacktrace you got :)
Oleg Barenboim
@chessbyte
Jan 17 2018 18:50
stupid question on the require leak -- did someone validate it on Linux?
Jason Frey
@Fryguy
Jan 17 2018 18:51
Instruments2 2018-01-17 13-50-55.png
Nick LaMuro
@NickLaMuro
Jan 17 2018 18:51
my tests have all been on Linux or a Linux VM running via vagrant
Oleg Barenboim
@chessbyte
Jan 17 2018 18:51
want to ensure we are not chasing a Mac-only issue
good - thanks!
Joe Rafaniello
@jrafanie
Jan 17 2018 18:53
Good point @chessbyte, yes, I am trying to only do things on mac after having enough confidence that it behaves the same way on linux
Jason Frey
@Fryguy
Jan 17 2018 18:53
yeah, I did Mac only to verify it wasn't Linux-only
happens on both envs
Oleg Barenboim
@chessbyte
Jan 17 2018 18:53
perfect -- carry on then
Joe Rafaniello
@jrafanie
Jan 17 2018 18:54
ok, so rb_feature_p is called from search_required where ruby looks up the existing loaded features for the thing we're about to require
it makes sense why that's enormous in our app since loaded features is huge compared to a vanilla rails app
Nick LaMuro
@NickLaMuro
Jan 17 2018 18:54

@chessbyte That said, the results after applying the patch I made to MiqSystem.num_cpus on (most of) an appliance can be found here:

https://gitter.im/ManageIQ/manageiq/performance?at=5a5e229c5ade18be398240c7

It is still commented heavily in both cases, so a lot of the functionality isn't running (working on getting results with that), but what was shown so far was a night and day change.

Jason Frey
@Fryguy
Jan 17 2018 18:55
but it should technically only get called once
the fact that it's called over and over is weird
or does it check loaded features over and over (to see if it's already loaded)
Joe Rafaniello
@jrafanie
Jan 17 2018 18:56
@Fryguy the sampling on osx and @NickLaMuro's gdb goodness seem to show it's pounding loaded features looking for matches
Jason Frey
@Fryguy
Jan 17 2018 18:56
right
mine too
Joe Rafaniello
@jrafanie
Jan 17 2018 18:56
let me get you a link
Jason Frey
@Fryguy
Jan 17 2018 18:56
no need...I have the c source in front of me
Oleg Barenboim
@chessbyte
Jan 17 2018 18:57
if I were the coder, I would expand the path of the gem, look it up in a hash of required_gems, and if it is not there, mark it and load it in an ensure block
Jason Frey
@Fryguy
Jan 17 2018 18:57
they have their own fast-lookup logic that is not a Hash (I don't remember specific reasons why they explicitly don't use a hash)
but it does use an index-like approach
Oleg Barenboim
@chessbyte
Jan 17 2018 18:59
OMG -- I cannot look at C code after a decade of Ruby ;-)
Jason Frey
@Fryguy
Jan 17 2018 19:00
I know right?
Oleg Barenboim
@chessbyte
Jan 17 2018 19:00
so freaking ugly
Jason Frey
@Fryguy
Jan 17 2018 19:00
after looking at this C code and the stack trace, I think we can do a vanilla rails reproducer by having at least 1 Pathname in the autoload-paths
or perhaps 1 Pathname in the loaded features
or in load path
Oleg Barenboim
@chessbyte
Jan 17 2018 19:01
so you think the leak is because of Pathname expansion?
Jason Frey
@Fryguy
Jan 17 2018 19:01
yes
rb_get_expanded_load_path is in the stack trace of the leaks
and I'm wondering if vanilla rails does or doesn't go down that path
@jrafanie and @NickLaMuro has the same/similar conclusion
Oleg Barenboim
@chessbyte
Jan 17 2018 19:02
great -- so worst case, we can monkey-patch require to fix the issue (at least on MiqServer)?
Jason Frey
@Fryguy
Jan 17 2018 19:02
maybe?
Oleg Barenboim
@chessbyte
Jan 17 2018 19:03
best case is to fix the leak in Ruby/Rails
Jason Frey
@Fryguy
Jan 17 2018 19:03
we could also just not use Pathname in load_path
I don't think we are at that point to be sure yet thogh
Oleg Barenboim
@chessbyte
Jan 17 2018 19:03
oh - pre-expand it before putting on load_path?
Jason Frey
@Fryguy
Jan 17 2018 19:03
yeah
Oleg Barenboim
@chessbyte
Jan 17 2018 19:03
roger
Joe Rafaniello
@jrafanie
Jan 17 2018 19:03
Note, I tried putting .to_s in our autoload paths lines in application.rb to no avail
Jason Frey
@Fryguy
Jan 17 2018 19:04
I don't think it's autoload_paths..I think it's LOAD_PATH
how many Pathnames are in your LOAD_PATH?
Joe Rafaniello
@jrafanie
Jan 17 2018 19:06
we have 12 in our load path
Jason Frey
@Fryguy
Jan 17 2018 19:07
O_O the magic number
I bet if we change those to be regular strings the leak goes away
they all look like they come from autoload-paths I think
Joe Rafaniello
@jrafanie
Jan 17 2018 19:09
some are from engines
irb(main):010:0> $LOAD_PATH.select {|p| p.class == Pathname}
=> [#<Pathname:/Users/joerafaniello/Code/manageiq/app/models/aliases>, #<Pathname:/Users/joerafaniello/Code/manageiq/app/models/mixins>, #<Pathname:/Users/joerafaniello/Code/manageiq/lib>, #<Pathname:/Users/joerafaniello/Code/manageiq/lib/services>, #<Pathname:/Users/joerafaniello/.gem/ruby/2.5.0/bundler/gems/manageiq-ui-classic-d809933336be/app/controllers/mixins>, #<Pathname:/Users/joerafaniello/.gem/ruby/2.5.0/bundler/gems/manageiq-ui-classic-d809933336be/lib>, #<Pathname:/Users/joerafaniello/.gem/ruby/2.5.0/bundler/gems/manageiq-api-fced14cbd7a2/app/controllers/api>, #<Pathname:/Users/joerafaniello/.gem/ruby/2.5.0/bundler/gems/manageiq-api-fced14cbd7a2/lib/services>, #<Pathname:/Users/joerafaniello/.gem/ruby/2.5.0/bundler/gems/manageiq-api-fced14cbd7a2/lib>, #<Pathname:/Users/joerafaniello/.gem/ruby/2.5.0/bundler/gems/manageiq-automation_engine-1a3c52726425/app/models/mixins>, #<Pathname:/Users/joerafaniello/.gem/ruby/2.5.0/bundler/gems/manageiq-automation_engine-1a3c52726425/lib/miq_automation_engine>, #<Pathname:/Users/joerafaniello/.gem/ruby/2.5.0/bundler/gems/manageiq-automation_engine-1a3c52726425/lib/miq_automation_engine/engine>]
Jason Frey
@Fryguy
Jan 17 2018 19:09
right
If you change all of those, does the leak go away?
Joe Rafaniello
@jrafanie
Jan 17 2018 19:10
Right, so my to_s test drops that number to 8
Jason Frey
@Fryguy
Jan 17 2018 19:10
ah ok
Joe Rafaniello
@jrafanie
Jan 17 2018 19:16
OMG
Jason Frey
@Fryguy
Jan 17 2018 19:16
?
Joe Rafaniello
@jrafanie
Jan 17 2018 19:16
138 MB and stable
Jason Frey
@Fryguy
Jan 17 2018 19:16
:trophy:
I can't seem to infect a vanilla rails ruby app with Pathnames though
Joe Rafaniello
@jrafanie
Jan 17 2018 19:17
yeah
Jason Frey
@Fryguy
Jan 17 2018 19:17
can you add some autoload-paths with Pathnames to your vanilla rails app?
Joe Rafaniello
@jrafanie
Jan 17 2018 19:18
I basically put .to_s on the end of all << autoload_paths calls in application.rb and the engines in api, ui-classic, and automation engine
Jason Frey
@Fryguy
Jan 17 2018 19:18
yup
Joe Rafaniello
@jrafanie
Jan 17 2018 19:18
diff --git a/config/application.rb b/config/application.rb
index caacdc1dba..88d1c7828e 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -90,15 +90,15 @@ module Vmdb
     # Customize any additional options below...

     config.autoload_paths += config.eager_load_paths
-    config.autoload_paths << Rails.root.join("app", "models", "aliases")
-    config.autoload_paths << Rails.root.join("app", "models", "mixins")
-    config.autoload_paths << Rails.root.join("lib", "miq_automation_engine", "models")
-    config.autoload_paths << Rails.root.join("lib", "miq_automation_engine", "models", "mixins")
-    config.autoload_paths << Rails.root.join("app", "controllers", "mixins")
-    config.autoload_paths << Rails.root.join("lib")
-    config.autoload_paths << Rails.root.join("lib", "services")
-
-    config.autoload_once_paths << Rails.root.join("lib", "vmdb", "console_methods.rb")
+    config.autoload_paths << Rails.root.join("app", "models", "aliases").to_s
+    config.autoload_paths << Rails.root.join("app", "models", "mixins").to_s
+    config.autoload_paths << Rails.root.join("lib", "miq_automation_engine", "models").to_s
+    config.autoload_paths << Rails.root.join("lib", "miq_automation_engine", "models", "mixins").to_s
+    config.autoload_paths << Rails.root.join("app", "controllers", "mixins").to_s
+    config.autoload_paths << Rails.root.join("lib").to_s
+    config.autoload_paths << Rails.root.join("lib", "services").to_s
+
+    config.autoload_once_paths << Rails.root.join("lib", "vmdb", "console_methods.rb".to_s).to_s
Jason Frey
@Fryguy
Jan 17 2018 19:18
that is exactly what I was thinking
and that means 0 in LOAD_PATH, right?
Joe Rafaniello
@jrafanie
Jan 17 2018 19:18
yup
irb(main):001:0> $LOAD_PATH.select {|p| p.class == Pathname}
=> []
Jason Frey
@Fryguy
Jan 17 2018 19:19
and then it goes down the same "normal" rb_find_file_path route?
Joe Rafaniello
@jrafanie
Jan 17 2018 19:19
let me sample it
Jason Frey
@Fryguy
Jan 17 2018 19:19
instead of rb_feature_p
Joe Rafaniello
@jrafanie
Jan 17 2018 19:20
correct
that's it
Jason Frey
@Fryguy
Jan 17 2018 19:20
super interesting
Joe Rafaniello
@jrafanie
Jan 17 2018 19:20
yeah, very cool
This explains why it's been getting worse... more extracted engines with more pathnames on the $PATH
Jason Frey
@Fryguy
Jan 17 2018 19:20
I really want to infect a Rails app and/or Ruby app, so we have a reproducer for those core teams
Though core ManageIQ has had Pathname in autoload_paths for a LONG time
Joe Rafaniello
@jrafanie
Jan 17 2018 19:24
ok, I'll try infecting a vanilla rails app too
Jason Frey
@Fryguy
Jan 17 2018 19:26
care to share :jeans: ? this is exciting haha
Joe Rafaniello
@jrafanie
Jan 17 2018 19:28
doh
I'll pretend I didn't already run it and know the results
Jason Frey
@Fryguy
Jan 17 2018 19:28
LOOL
share anyways...I bet we can get a Ruby reproducer real soon
(yes I said "anyways" just for you)
Nick LaMuro
@NickLaMuro
Jan 17 2018 20:48
Script for replicating a ruby leak with pure ruby:
require 'pathname'
require 'fileutils'

puts "Process: #{Process.pid}"

paths = %w[
  dir_1
  dir_2
  dir_3
  dir_4
]


paths.each do |path|
  Dir.mkdir path unless Dir.exists? path
  $LOAD_PATH.unshift(Pathname.new path)
end

FileUtils.touch "#{paths.last}/empty.rb"


dot      = "."
filename = "empty"

1500.times { 1500.times { require filename }; print dot; GC.start; }
Keenan Brock
@kbrock
Jan 17 2018 20:49
nice
Jason Frey
@Fryguy
Jan 17 2018 20:50
this is easily the coolest thing we've found in a long time haha
Nick LaMuro
@NickLaMuro
Jan 17 2018 20:50
Jason Frey
@Fryguy
Jan 17 2018 20:50
totally
Keenan Brock
@kbrock
Jan 17 2018 20:52
Nick - you forgot to freeze the strings. THAT is why you're leaking ;)
:bomb: :runner:
ok, real statement
what versions of ruby show this as a bug?
Jason Frey
@Fryguy
Jan 17 2018 20:53
anything < 2.5
Nick LaMuro
@NickLaMuro
Jan 17 2018 20:53
^ this
Jason Frey
@Fryguy
Jan 17 2018 20:53
not sure about 2.2 (didn't try it)
Keenan Brock
@kbrock
Jan 17 2018 20:53
is the fix as easy as it looks?
Jason Frey
@Fryguy
Jan 17 2018 20:53
Smaller script:
require 'pathname'
require 'fileutils'

puts Process.pid

Dir.mkdir("foo") unless Dir.exists?("foo")
$LOAD_PATH.unshift(Pathname.new("foo"))

FileUtils.touch("empty.rb")
1500.times { 1500.times { require "empty" }; print "."; GC.start; }
Nick LaMuro
@NickLaMuro
Jan 17 2018 20:54
@kbrock basically a .to_s on all calls to autoload_paths
which include some provider plugins
Jason Frey
@Fryguy
Jan 17 2018 20:54
yeah .to_s is a workaround for now
basically to avoid putting Pathname objects into the LOAD_PATH
Nick or LJ will open an issue on Ruby bug tracker
Keenan Brock
@kbrock
Jan 17 2018 20:55
how many PRs for our code base?
Jason Frey
@Fryguy
Jan 17 2018 20:55
3 or 4
Keenan Brock
@kbrock
Jan 17 2018 20:55
nice
(per version I presume)
Jason Frey
@Fryguy
Jan 17 2018 20:55
yeah, we'll backport all the way
Keenan Brock
@kbrock
Jan 17 2018 20:56
are initializers run after all the engines and gems are included?
Gregg Tanzillo
@gtanzillo
Jan 17 2018 20:56
Amazing!!
Keenan Brock
@kbrock
Jan 17 2018 20:56
can we just fixup the paths in an initializer on our appliances?
(short term fix)
Jason Frey
@Fryguy
Jan 17 2018 20:57
Rails itself modifies the LOAD_PATH with an initializer, so it's kind of complicated
Jason Frey
@Fryguy
Jan 17 2018 20:57
much easier to just tack on the .to_s in a dozen lines or so
Keenan Brock
@kbrock
Jan 17 2018 20:59
You get a score of <bad>
We have a few ways to help increase your retirement score, like working for a few more years.
--Fidelity
Gee thanks. feels like the performance people saying: "Just buy more hardware"
I was thinking you could write an initializer script that loops and to_s all the entries in the $LOAD_PATH
Jason Frey
@Fryguy
Jan 17 2018 21:03
$LOAD_PATH is not a "normal" Array, so not sure how you would do that
like once it's in the LOAD_PATH, I don't think you can change it
Keenan Brock
@kbrock
Jan 17 2018 21:04
the uniq! seemed to suggest it is mutable
Jason Frey
@Fryguy
Jan 17 2018 21:04
but it probably has it's own implementation of uniq!
Keenan Brock
@kbrock
Jan 17 2018 21:04
you could unshift everything to_s and uniq! the thing?
hmm
anyway. has anyone tried the original code with this change?
Nick LaMuro
@NickLaMuro
Jan 17 2018 21:06
@kbrock are you saying on an appliance?
Joe Rafaniello
@jrafanie
Jan 17 2018 21:06
@kbrock I manually changed the engines and application.rb to do .to_s on the autoload_path lines and it no longer leaks
Nick LaMuro
@NickLaMuro
Jan 17 2018 21:07
if so, working on it... but also that ^
Keenan Brock
@kbrock
Jan 17 2018 21:08
thnx @jrafanie / nick
Joe Rafaniello
@jrafanie
Jan 17 2018 21:09
@kbrock to confuse myself early this morning, I had changed the .to_s on just the application.rb autoload_paths but didn't do the engines and it was still leaking so I thought "it can't be that"
Keenan Brock
@kbrock
Jan 17 2018 21:09
of course :(
Joe Rafaniello
@jrafanie
Jan 17 2018 21:09
We've found even with just a single pathname, it leaks badly
Gregg Tanzillo
@gtanzillo
Jan 17 2018 21:09
Awesome work @jrafanie and @NickLaMuro :clap: :sparkles:
Jason Frey
@Fryguy
Jan 17 2018 21:10
yup single pathname is in the pure Ruby reproducer :point_up: January 17, 2018 3:53 PM
Joe Rafaniello
@jrafanie
Jan 17 2018 21:11
it was a team effort... lots of dumping ideas and others commenting on it...
All that for a 7 line reproducer :point_up: January 17, 2018 3:53 PM
Oleg Barenboim
@chessbyte
Jan 17 2018 21:12
sweet!!! great job, team!! :beers: on me
Nick LaMuro
@NickLaMuro
Jan 17 2018 21:12
I'll take a :droplet:
Keenan Brock
@kbrock
Jan 17 2018 21:40
? take a drink?
Nick LaMuro
@NickLaMuro
Jan 17 2018 21:41
(water)
Nick LaMuro
@NickLaMuro
Jan 17 2018 22:03

@kbrock @jrafanie @Fryguy @dmetzger57 Relevant PRs for the leak workaround:

ManageIQ/manageiq#16837
ManageIQ/manageiq-api#288
ManageIQ/manageiq-automation_engine#146
ManageIQ/manageiq-ui-classic#3266

Keenan Brock
@kbrock
Jan 17 2018 22:03
thnx
Nick LaMuro
@NickLaMuro
Jan 17 2018 22:06
stepping away for a bit, AFK
Keenan Brock
@kbrock
Jan 17 2018 22:06
lookin' good
Keenan Brock
@kbrock
Jan 17 2018 22:14
Looking at ManageIQ/manageiq#16807 -- It makes me think we should be generating ActiveSupport::Notifications for these as well
Dennis Metzger
@dmetzger57
Jan 17 2018 22:31
thanks @NickLaMuro @jrafanie @Fryguy
Joe Rafaniello
@jrafanie
Jan 17 2018 22:36
looks good @NickLaMuro. Do you want to open the ruby bug now or tomorrow?
Nick LaMuro
@NickLaMuro
Jan 17 2018 22:49
@jrafanie up to you, I am available if you wanted to do it tonight yet, but don't need to force it
Joe Rafaniello
@jrafanie
Jan 17 2018 23:08
yeah, we'll do it in the morning