These are chat archives for ManageIQ/manageiq/performance

25th
May 2016
Keenan Brock
@kbrock
May 25 2016 01:12
Has anyone looked into enabling query cache for each miq_queue job?
I turned it on for a refresh of 17 vms:
ms ms- sql sqlms sqlrows comments
6,168.3 1,334 241.5 868 refresh no cache
6,669.1 1,244 235.5 651 refresh cache
I can look into calling that for 1,000 but 10% faster, 10% fewer queries, and 25% fewer records brought back
Keenan Brock
@kbrock
May 25 2016 01:29
@Fryguy any thoughts on running all queue entries through AR cache (like middle ware does for all ui screens)?
Jason Frey
@Fryguy
May 25 2016 01:29
When your d
No phone, I wasn't ready to send that
Keenan Brock
@kbrock
May 25 2016 01:29
lol
Chris Arcand
@chrisarcand
May 25 2016 01:30
Say no to phones
Jason Frey
@Fryguy
May 25 2016 01:30
When you say middleware, do you mean rack?
Keenan Brock
@kbrock
May 25 2016 01:30
yea
Jason Frey
@Fryguy
May 25 2016 01:30
It is a very interesting idea
Keenan Brock
@kbrock
May 25 2016 01:30
EmsRefresh.refresh(ExtManagementSystem.first) } ; nil
# vs
ActiveRecord::Base.connection.cache { EmsRefresh.refresh(ExtManagementSystem.first) } } ; nil
Jason Frey
@Fryguy
May 25 2016 01:31
I'm concerned it will break certain things though, like batching stuff like purging
Keenan Brock
@kbrock
May 25 2016 01:31
rails 2 (and 3) used to work that way
Jason Frey
@Fryguy
May 25 2016 01:31
I don't remember Rails 2 or 3 doing that
At least outside of the UI
Keenan Brock
@kbrock
May 25 2016 01:31
ooh
maybe
ms ms- sql sqlms sqlrows comments
161,356.2 53,004 11,742.5 31,869 refresh 1,000 no cache
157,100.3 49,208 9,635.7 24,545 refresh 1,000 cache
hmm
lost something there
Jason Frey
@Fryguy
May 25 2016 01:32
Extra col in first row
What is the % savings
Doesn't seem like much savings time wise
Keenan Brock
@kbrock
May 25 2016 01:33
this is running on a local database - once you run on a remote database ...
and sql rows
Jason Frey
@Fryguy
May 25 2016 01:34
Refresh is a bad candidate because of inserts which aren't cashable
Keenan Brock
@kbrock
May 25 2016 01:34
that is how much memory you are using up in memory :)
Jason Frey
@Fryguy
May 25 2016 01:34
True
Keenan Brock
@kbrock
May 25 2016 01:34
right
refresh is blowing out the cache intentionally
it does (true) everywhere
AND
22% less data over the wire
change is ~2 lines of code
for every worker?
7% fewer queries - expecting better
Keenan Brock
@kbrock
May 25 2016 02:09
@akrzos this one is for you: ManageIQ/manageiq#8948
Keenan Brock
@kbrock
May 25 2016 02:38
@dmetzger57 lol - have I told you before that I don't like count(*)? (just a complaint as I'm trying to trim down this page :) )
Dennis Metzger
@dmetzger57
May 25 2016 02:39
@kbrock never heard you say that ………..
Keenan Brock
@kbrock
May 25 2016 02:39
so I'm on this "slow updates page"
32 queries
7 rbac calls
for a user typing in 1 character into the ui
Dennis Metzger
@dmetzger57
May 25 2016 02:39
ouch
Keenan Brock
@kbrock
May 25 2016 02:40
I guess only 4 of them are count(*) - and they are trivial at that
ooooh - it is processing rbac for each of the fields entered, not just the changed one
I totally need to write a routine that takes 5 sql stacktraces and merges them together, so you get a single stack trace and the sql littered throughout
Dennis Metzger
@dmetzger57
May 25 2016 02:42
this is for the bz from Dan?
Keenan Brock
@kbrock
May 25 2016 02:42
yea
ms ms- sql sqlms sqlrows comments
490.8 32 10.4 25 /vm_infra/prov_field_changed/new
490.8 39.7 POST http://localhost:3000/vm_infra/prov_field_changed/new
451.1 178.2 19 6.6 21 .Executing action: prov_field_changed
40.6 38.0 2 0.6 ..Rbac.search
43.2 40.5 3 0.9 1 ..Rbac.search
43.1 39.8 1 0.3 ..Rbac.search
40.5 37.9 3 0.8 3 ..Rbac.search
31.0 29.8 1 0.3 ..Rbac.search
29.7 27.5 2 0.7 ..Rbac.search
28.7 27.4 1 0.2 ..Rbac.search
16.0 16.0 ..Rendering: layouts/_flash_msg
there, that is better
better yet, all the fields are blank
so no need to run rbac on all of those fields when the values are blank. (and no drop downs)
I'll have to hit GM on this tomorrow
Dennis Metzger
@dmetzger57
May 25 2016 02:44
think you should shutdown for the night
Keenan Brock
@kbrock
May 25 2016 02:45
yea - I've probably hit enough threads
I'm really excited about the sql cache
want to see what alex does with that
think we could get a 10% 2% performance savings (and possible memory drop) right across the board (hopeful... and doubtful ;) )
==> ManageIQ/manageiq#8948
Dennis Metzger
@dmetzger57
May 25 2016 02:46
will be interesting to see how this one plays out
Keenan Brock
@kbrock
May 25 2016 02:47
there I go. I was saying 10% improvement, when it is only showing 2% for my test :(
Dennis Metzger
@dmetzger57
May 25 2016 02:47
the eternal optimist…..
Keenan Brock
@kbrock
May 25 2016 02:47
cracktimist
Dennis Metzger
@dmetzger57
May 25 2016 02:47
yea, not really optimist
Keenan Brock
@kbrock
May 25 2016 02:47
lol
I'll follow your wisdom... night
Dennis Metzger
@dmetzger57
May 25 2016 02:48
‘night
Alex Krzos
@akrzos
May 25 2016 11:22
@kbrock Interesting, we could patch an appliance or two and run through some workloads or benchmarks and see the results with refresh or others
Keenan Brock
@kbrock
May 25 2016 13:13
@akrzos we'd have to figure out if it changed the values, but curious how big of a change it would make
Jason Frey
@Fryguy
May 25 2016 14:14
@kbrock So in looking at your finding on serializing AR objects on the queue, I found that with the new version of Rails, and in particular, because of the new attributes API, a serialized ActiveRecord object is significantly larger
So that would also affect session, and probably a number of other places as well, where we might serialize an AR object
cc @jrafanie ^
An EmsCluster with no associations is 50kb as yml (17kb as marshal dumped)
Joe Rafaniello
@jrafanie
May 25 2016 14:17
lol, can you people keep your subject in one room?
Jason Frey
@Fryguy
May 25 2016 14:18
This is a performance problem, so i'm telling the performance people
Joe Rafaniello
@jrafanie
May 25 2016 14:18
:trollface:
Jason Frey
@Fryguy
May 25 2016 14:18
anyways :P
Here is a dump of the cluster I found in my MiqQueue: gist
notice the AttributeSet gets serialized (not sure it should be serialized, so maybe that's something we should propose as a patch to Rails)
Keenan Brock
@kbrock
May 25 2016 14:57
of course all the virtual attributes acting like real attributes will blow the thing up
@Fryguy maybe we should modify serialze to skip virtual columns?
Jason Frey
@Fryguy
May 25 2016 14:58
I'm wondering if ActiveRecord should be serializing those things at all
I'm sure it was just an oversight on their part
Keenan Brock
@kbrock
May 25 2016 14:59
aah, it is serializing cluster
Jason Frey
@Fryguy
May 25 2016 15:00
yes...ah yeah, my gist shows the entire MiqQueue#args, but 90% of it is the cluster
Keenan Brock
@kbrock
May 25 2016 15:00
ok, i see - you taking that or should I?
Jason Frey
@Fryguy
May 25 2016 15:01
which part? @gmcculloug is already working on the issue you found during refresh
Keenan Brock
@kbrock
May 25 2016 15:02
PR for rails
Jason Frey
@Fryguy
May 25 2016 15:02
ah...you can take that if you want
Keenan Brock
@kbrock
May 25 2016 15:02
so you said AttributeSet serializes the AR record to yaml?
Jason Frey
@Fryguy
May 25 2016 15:03
you can see all the data right there under the ems_cluster
Keenan Brock
@kbrock
May 25 2016 15:03
I see the bug
Jason Frey
@Fryguy
May 25 2016 15:03
actually, now that I look at it I'm even more concerned
Keenan Brock
@kbrock
May 25 2016 15:03
curious where in rails to look for the fix
yes
Jason Frey
@Fryguy
May 25 2016 15:03
because does ActiveRecord actually call all those methods to get the values?
some of those virtual columns can be really expensive
Keenan Brock
@kbrock
May 25 2016 15:04
right
a. we need to remove ems_cluster for everyone
b. we need to remove the virtual columns (as a hack/patch) for our code base
Jason Frey
@Fryguy
May 25 2016 15:05

does ActiveRecord actually call all those methods to get the values?

No, I don't think it does on looking again, because region_number doesn't have a value and if it were called it would have a value

@kbrock Also see that the serialization contains the delegate_hash, which includes before_type_cast and after_type_cast values
Keenan Brock
@kbrock
May 25 2016 15:07
yea
um, I have too many balls in the air right now. I'll put a note in my queue, but it won't get love any time soon
Joe Rafaniello
@jrafanie
May 25 2016 15:07

I'm wondering if ActiveRecord should be serializing those things at all

Is there a reasonable use case for serializing AR objects? If so, that's the place to open the PR/issue on rails against

Jason Frey
@Fryguy
May 25 2016 15:07
We can probably just open an issue on Rails itself that the new attributes API is causing serialization of AR instances to explode in size
@jrafanie I'm sure there are, but I don't know them offhand
@kbrock I'll take care of opening the Rails issue
I think I know how to word it in the right way :)
Keenan Brock
@kbrock
May 25 2016 15:13
well, the serialization does a deep serialize. Wonder if there is a way to say: "please, not every object you have ever met"
Joe Rafaniello
@jrafanie
May 25 2016 15:15
yeah, what did that ems cluster look like before vs. now? Is the data the same but is now encapsulated in the new attributes api objects?
Jason Frey
@Fryguy
May 25 2016 15:16
oh if I just do EmsCluster.first.to_yaml it's that big
I updated my gist to just have the EmsCluster and nothing else
Keenan Brock
@kbrock
May 25 2016 15:16
yes, it is all the objects it knows
Lucy Fu
@lfu
May 25 2016 15:32
about the DB table bloat, it is caused by setting the column type = ‘EmsEvent'
if only schema change, the size does not increase.
if i run the same migration that renames the table and sets the type column repeatedly after migrate down first, every time the table size increased by about 9GB.
i think that the size it started with.
Jason Frey
@Fryguy
May 25 2016 15:34
@lfu Is it 9GB bigger after a full vacuum as well?
Lucy Fu
@lfu
May 25 2016 15:39
running the vacuum
Keenan Brock
@kbrock
May 25 2016 15:42
+1 on vacuum
may need to drop and recreate that index
Lucy Fu
@lfu
May 25 2016 15:45
no index on that table
Keenan Brock
@kbrock
May 25 2016 15:48
aah
@lfu when you update a table, you insert and delete
if you update multiple times, you insert and delete multiple times
Jason Frey
@Fryguy
May 25 2016 15:52
@lfu There aren't indexes on that table? I'm very surprised at that
Keenan Brock
@kbrock
May 25 2016 15:52
do we have a general 'vacuum the whole house' rake task?
maybe we need to put that as part of our upgrade instructions
@lfu did vacuum help you?
Jason Frey
@Fryguy
May 25 2016 15:53
@lfu
[10] pry(main)> ActiveRecord::Base.connection.indexes("event_streams").map(&:name)
=> ["index_event_streams_on_chain_id_and_ems_id",
 "index_event_streams_on_dest_host_id",
 "index_event_streams_on_dest_vm_or_template_id",
 "index_event_streams_on_ems_cluster_id",
 "index_event_streams_on_ems_id",
 "index_event_streams_on_event_type",
 "index_event_streams_on_host_id",
 "index_event_streams_on_timestamp",
 "index_event_streams_on_vm_or_template_id"]
Keenan Brock
@kbrock
May 25 2016 15:54
    t.index ["chain_id", "ems_id"], name: "index_event_streams_on_chain_id_and_ems_id", using: :btree
    t.index ["dest_host_id"], name: "index_event_streams_on_dest_host_id", using: :btree
    t.index ["dest_vm_or_template_id"], name: "index_event_streams_on_dest_vm_or_template_id", using: :btree
    t.index ["ems_cluster_id"], name: "index_event_streams_on_ems_cluster_id", using: :btree
    t.index ["ems_id"], name: "index_event_streams_on_ems_id", using: :btree
    t.index ["event_type"], name: "index_event_streams_on_event_type", using: :btree
    t.index ["host_id"], name: "index_event_streams_on_host_id", using: :btree
    t.index ["timestamp"], name: "index_event_streams_on_timestamp", using: :btree
    t.index ["vm_or_template_id"], name: "index_event_streams_on_vm_or_template_id", using: :btree
Jason Frey
@Fryguy
May 25 2016 15:54
@kbrock Not a rake task, but I believe it's part of the CloudForms maintenance documentation (which is part of the upgrade doc as well)
Keenan Brock
@kbrock
May 25 2016 15:54
good
Lucy Fu
@lfu
May 25 2016 16:01
ok, after vacuum, the size went down to 8889MB, original is 8855MB
so the migration does not touch the index
regarding update_all and batch, seems update_all is better.
it constructs a single SQL UPDATE statement and sends it straight to the database. It does not instantiate the involved models and it does not trigger Active Record callbacks or validations.
Lucy Fu
@lfu
May 25 2016 16:08
in testing, it took about ~15 minutes while batch took about 2 hours.
find_each and update_attributes
== 20150806211453 RenameEmsEventTableToEventStream: migrated (7052.8387s) =====
find_each and update_attribute
== 20150806211453 RenameEmsEventTableToEventStream: migrated (7500.8826s) =====
Greg McCullough
@gmcculloug
May 25 2016 16:09
@lfu Is the outcome of all this show that the migration does not need to be changed? Only a db vacuum after migration?
Lucy Fu
@lfu
May 25 2016 16:09
update_all got almost the same result as using raw sql
Keenan Brock
@kbrock
May 25 2016 16:10
@lfu that is what is referred to as N+1
Jason Frey
@Fryguy
May 25 2016 16:10
it was update_all all along, though right?
Lucy Fu
@lfu
May 25 2016 16:10
yeah, looks like vacuum trim down the size
the migration is using update_all
Jason Frey
@Fryguy
May 25 2016 16:11
also, yes update_all will technically always be faster...you batch because of other factors like memory or disk size
classic memory/speed/disk tradeoff
can you try the migration on an appliance (as opposed to your local system which likely has a lot more memory?)
Lucy Fu
@lfu
May 25 2016 16:12
Alex mentioned with an appliance with default 8GB mem, it came to a crawl
Greg McCullough
@gmcculloug
May 25 2016 16:14
@lfu for that migration specifically or the other one mentioned in the ticket?
Lucy Fu
@lfu
May 25 2016 16:14
all migration together
Keenan Brock
@kbrock
May 25 2016 16:27
the update_all should use a lot less data
@lfu is it using the callbacks?
Greg McCullough
@gmcculloug
May 25 2016 16:29
@kbrock @Fryguy does the table size increase make sense? (even if a vacuum fixes it) seems like it could still cause issues.
Jason Frey
@Fryguy
May 25 2016 16:30
@kbrock update_all is the thing causing the issue
Lucy Fu
@lfu
May 25 2016 16:30
@kbrock don’t see callbacks in the model
Jason Frey
@Fryguy
May 25 2016 16:31
@lfu it should be a stub model anyway, right?
@kbrock cause it's updating 7mil records in one transaction
Lucy Fu
@lfu
May 25 2016 16:31
@Fryguy yeah, stub model
Jason Frey
@Fryguy
May 25 2016 16:32
batching (though slower locally where you have a ton of memory), might be faster on a memory-tight appliance
@lfu Also, batch size makes a difference...you can probably do something like 100_000 records in a batch
maybe even more
Lucy Fu
@lfu
May 25 2016 16:33
regarding the data, i know we need to keep them. but do we need to keep all of them?
Jason Frey
@Fryguy
May 25 2016 16:34
@lfu yes...retention of events is customer driven
they may want it for auditing purposes
however on that note, they should be purged before doing the upgrade
default retention is 6 months of data (which 7 million sounds reasonable to me)
Keenan Brock
@kbrock
May 25 2016 16:46
@Fryguy duh - stub model. thanks ( /me :dork: )
would be nice to not have to download all these records
Jason Frey
@Fryguy
May 25 2016 16:48
update_all doesn't download any records
Keenan Brock
@kbrock
May 25 2016 16:48
@lfu what migrationis this again?
but batches tends to download them
Jason Frey
@Fryguy
May 25 2016 16:48
it is basically straight UPDATE table_name SET col=val
Keenan Brock
@kbrock
May 25 2016 16:49
+1
UPDATE table_name set col=val where col=old_val limit 100_000
you can do that in batches
Jason Frey
@Fryguy
May 25 2016 16:49
exactly
loop until no records updated
Keenan Brock
@kbrock
May 25 2016 16:49
remember having something like this
with "windowing"
Lucy Fu
@lfu
May 25 2016 16:50
20150806211453_rename_ems_event_table_to_event_stream.rb @kbrock
Jason Frey
@Fryguy
May 25 2016 16:51
which in AR::Relation is more or less
update_count = nil
until update_count == 0
  update_count = EventStream.limit(100_000).update_all(:key => val)
end
yeah I one of the purgers does something similar
Lucy Fu
@lfu
May 25 2016 16:52
how would this affect the table size?
that is the part i still don’t get it
Jason Frey
@Fryguy
May 25 2016 16:53
table size is independent of this...what this prevents is the database transaction log from growing inordinately big and from postgres having to keep tons of records in memory at once
I don't really understand why table size is blowing up like it does, and vacuum fixes it...I would expect autovacuum to fix it
Greg McCullough
@gmcculloug
May 25 2016 16:54
@lfu I think the only issue we are dealing with in this migration is the table size, correct?
Jason Frey
@Fryguy
May 25 2016 16:54
I thought it took like 7 hours to complete
or something crazy
Lucy Fu
@lfu
May 25 2016 16:55
most of the 7 hours was taken but MiqReport migration, for EmsEvent it is the size issue
@gmcculloug that is correct
Greg McCullough
@gmcculloug
May 25 2016 16:57
@Fryguy the original BZ was reporting multiple issues and we were able to identify the migrations responible for each. table size = events. time/memory = reporting.
Jason Frey
@Fryguy
May 25 2016 16:57
oh ok...that's confusing :(
Greg McCullough
@gmcculloug
May 25 2016 16:57
the BZ was split so we could focus on each
Jason Frey
@Fryguy
May 25 2016 16:57
gotcha
Lucy Fu
@lfu
May 25 2016 16:58
@gmcculloug i was writing the same thing :)
Jason Frey
@Fryguy
May 25 2016 16:58
I still am curious if batching solves the size issue (or at least mitigates it)
Lucy Fu
@lfu
May 25 2016 16:58
it did not.
Keenan Brock
@kbrock
May 25 2016 16:58
in the end of the day you have to change the same number of records
Jason Frey
@Fryguy
May 25 2016 16:58
@lfu with which batch size?
Lucy Fu
@lfu
May 25 2016 16:58
size still bloated after 2 hours+
default 1000
Jason Frey
@Fryguy
May 25 2016 16:59
that's waaaaay too small for batching
Lucy Fu
@lfu
May 25 2016 16:59
i can try 100_000
Jason Frey
@Fryguy
May 25 2016 16:59
what I'm wondering is if the records update in some reasonable batch size that the autovacuumer can do its job more effectively
however, as an aside, maybe we can just chalk it up to documentation that the user has to vacuum after upgrade (I think we already recommend this anyway, somewhere)
Oleg Barenboim
@chessbyte
May 25 2016 17:00
DBA capabilities inside ManageIQ cannot come soon enough for my taste
Greg McCullough
@gmcculloug
May 25 2016 17:04
agree that documentation is the likely route. I asked lucy to update the BZ and look at the objects getting into the queue next to get a better handle on that issue. we can come back to this one if needed
Keenan Brock
@kbrock
May 25 2016 17:06
they probably hadn't thought to vacuum when the BZ was written
Dennis Metzger
@dmetzger57
May 25 2016 17:24
I recall Alex had a couple concerns when he put this ticket in, one being the amount of disk space required to perform the migration and the time to complete (over 5 hours I believe). Agreed, documenting for now it probably the best short term solution. Is the rule of thumb you need 2X the size of event table to be safe?
Greg McCullough
@gmcculloug
May 25 2016 17:28
the time/memory is still an issue and the original bug (BZ 1337159) is around to address that. not sure about rule of thumb or what is documented for db size during migration.
I would still like to understand why it increases so much but also need to get a handle on why we are putting active record objects on the queue. priorities. :sweat:
Dennis Metzger
@dmetzger57
May 25 2016 17:38
About that concern of running out of things to work on.....NOT
Joe Rafaniello
@jrafanie
May 25 2016 18:21
@dmetzger57 @akrzos @Fryguy and I got it. the bundler change caused SOME of the memory growth
bundler also changed:
-   1.10.6
+   1.11.2
I see 20-30 MB jumps upgrading from 1.10.6 -> 1.11.2 locally on osx
just loading rails console
(sorry, this is for the 5.5.0.13 -> 5.5.2.0 memory growth bug)
Dennis Metzger
@dmetzger57
May 25 2016 18:25
Which is the bulk of the base growth
Base == Boot Idle
Dennis Metzger
@dmetzger57
May 25 2016 18:30
@jrafanie is that 30MB PSS?
Joe Rafaniello
@jrafanie
May 25 2016 18:31
it's osx memory so who knows
I want to test this on linux next
bundler 1.10.6, 131MB (cfme 5.5.0.13)
bundler 1.11.2, 173-177MB (cfme 5.5.2.0)
bundler 1.12.4, 122-124 MB
Same Gemfile.lock, same ruby 2.2.0 on OSX, memory of bin/rails console as reported my osx's top
I ran all of them several times
Can we deploy a new 5.5.2.0 or a newer one that we have a baseline for?
Joe Rafaniello
@jrafanie
May 25 2016 18:37
we've fixed an issue with newer bundlers on 5.5.x but we'll need this change on 5.5 versions that don't have it
See ManageIQ/manageiq#8348 or just grab the patch: https://github.com/ManageIQ/manageiq/pull/8348.patch
Jason Frey
@Fryguy
May 25 2016 18:39
Is that what account for the drop back down in the charts?
between 5.5.4.0 and 5.5.4.1?
or are you saying that patch is needed beforehand?
Joe Rafaniello
@jrafanie
May 25 2016 18:43
Brandon backported that patch fairly recently on 5.5.x so you need that patch on appliances from versions before his backport
I'll look at when 5.5.4.1 was released, that was when it went back down
Jason Frey
@Fryguy
May 25 2016 18:44
yeah but what does that patch even do...that is, how does that drop the memory?
or does that just let us support newer bundlers (and that's all)
Joe Rafaniello
@jrafanie
May 25 2016 18:45
let me start over
you can't use newer bundlers without that patch
Jason Frey
@Fryguy
May 25 2016 18:46
oh ok...I think I understand now...I thought you were saying that fixed the memory issues :/
Joe Rafaniello
@jrafanie
May 25 2016 18:46
lol
that would be funny
trying more bundler versions
Alex Krzos
@akrzos
May 25 2016 18:50
@jrafanie @dmetzger57 Just getting back, was getting my intern all set up with his own appliance and collectd/graphite/grafana monitoring, he is all set now so lets see what i can help with
Joe Rafaniello
@jrafanie
May 25 2016 18:51
bundler 1.10.6, 131MB (cfme 5.5.0.13)
bundler 1.11.2, 173-177MB (cfme 5.5.2.0)
bundler 1.12.0, 173-174 MB
bundler 1.12.1, 121-124 MB
bundler 1.12.2, 121-123 MB
bundler 1.12.4, 122-124 MB
Jason Frey
@Fryguy
May 25 2016 18:51
well that looks like a simple fix ;)
Alex Krzos
@akrzos
May 25 2016 18:53
Well thats nice, I was curious about the 5.5.4.1/5.5.4.2 memory drop at idle
Jason Frey
@Fryguy
May 25 2016 19:01
haha, so I took a look at what's in bundler 1.12.1 that's different, and there aren't a lot of changes
but one particular update is they updated molinillo (the dependency tree resolver) from 0.4.3 to 0.4.5
and the BEST part is that the thing that changed in molinillo is specifically a memory performance fix PR that @jrafanie did
full-circle haha
Dennis Metzger
@dmetzger57
May 25 2016 19:05
😳
Joe Rafaniello
@jrafanie
May 25 2016 19:05
Oh look at that... I had no idea it duped objects that stayed referenced
Jason Frey
@Fryguy
May 25 2016 19:06
If I recall @jrafanie made the change for time reasons...I don't think he knew about the memory drop haha
Joe Rafaniello
@jrafanie
May 25 2016 19:06
I did the change because it was slowing down my console load ;-)
CocoaPods/Molinillo#39
LOL, I even concentrated on ruby prof (time measurements)
Jason Frey
@Fryguy
May 25 2016 19:08
I think this is officially @jrafanie's best performance fix ever :trophy:
"oh look I already fixed it in the future...how convenient"
Joe Rafaniello
@jrafanie
May 25 2016 19:16
yeah, upgrading from 1.11.2 -> 1.12.1 on @akrzos's appliance (rails console goes from 175 MB -> 136 MB RSS)
Joe Rafaniello
@jrafanie
May 25 2016 19:31
https://bugzilla.redhat.com/show_bug.cgi?id=1296695 is updated @akrzos @dmetzger57 We already shipped bundler 1.12.1+ in 5.5.4.1, not sure you want to close this as fixed since the code fix was already done
Alex Krzos
@akrzos
May 25 2016 19:53
So 5.5.4.1 drops in overall memory usage with a workload too so let me just quantify how much and I'll add my comments to that bz about when it was fixed
Joe Rafaniello
@jrafanie
May 25 2016 19:54
Now, onto 5.6 memory
I'm chatting with one of the devs on bundler RE: other opportunities
Note, 5.6 is forked workers so I have no idea how much of the shared memory from bundler is actually being copied from shared pages to private pages
Alex Krzos
@akrzos
May 25 2016 19:57
so Inventory+C&U for 4 hours on a medium provider is about ~500MiB used appliance memory difference between 5.5.3.4 and 5.5.4.1
medium provider == 1000vms
500 online for C&U collection
Dennis Metzger
@dmetzger57
May 25 2016 20:13
@akrzos that's with the default number of workers correct, not single worker per role?
Alex Krzos
@akrzos
May 25 2016 20:13
@dmetzger57 Yes
However when you compare to 5.5.0.13-2
5.5.4.1 is a few MiB higher (on the order of 13MiB)
That spreadsheet is where I'm compiling normalizing C&U workload with the different versions
so 5.5.4.1 drops memory usage back to near 5.5.0.13-2 usage levels when compared to 5.5.3.4
Joe Rafaniello
@jrafanie
May 25 2016 20:16
:+1:
Jason Frey
@Fryguy
May 25 2016 22:25
I opened the Rails issue about the serialization sizes... rails/rails#25145
Even if we don't serialize I don't think they should be that big