HomeMesosphereNo notifications. 4 unresolved issues.

Used `map` instead of `mapValues` in `RootGroup`.
ClosedAll Users

Authored by mpark on Dec 12 2016, 7:23 PM.

Details

Summary

Found this blog post shortly after our meeting: http://blog.bruchez.name/2013/02/mapmap-vs-mapmapvalues.html
I learned that by using mapValues, I'm actually continually constructing MappedValues instances on top of the original map. It seems like this is at least part of the memory leak that @jasongilanfarr was referring to. It's not a "leak" per-se, but we believe we continue to layer MappedValues after another on top of the original map.

When running the following bash script:

#!/bin/bash

for i in {1..300}; do
http POST :8080/v2/apps <<EOF
{
  "id" : "/$(($i % 17))/$(($i % 13))/$(($i % 11))/$(($i % 7))/simple-app-$i",
  "cmd": "sleep 1000",
  "instances": 1,
  "cpus": 0.01,
  "mem": 1,
  "disk": 0
}
EOF
done
  • On master, I was only able to get to about 150 before it basically stopped moving, and JMC claimed that I had about 1.2 million instances of Group being constructed already.
  • On master + RootGroup reverted, I got to 300, and had about 170,000 instances.
  • D320 + with this patch, I got to 300 and had about 39,000 instances.
Test Plan

sbt test

Diff Detail

Repository
rMARATHON marathon
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Changes from before your most recent comment are hidden. Show Older Changes
jeschkies accepted this revision.Dec 12 2016, 9:14 PM
jeschkies added a reviewer: jeschkies.
jeschkies added a subscriber: jeschkies.

Seems like a nice quick fix. I'd like to know what happens underneath. Could it be that the GC collects the old versions since no reference is pointing to them while mapValues is keeping references so the cannot get GC'ed?

This revision is now accepted and ready to land.Dec 12 2016, 9:14 PM
timcharper accepted this revision.Dec 12 2016, 9:52 PM

@jeschkies I think the hypothesis you propose is correct.

I’ve always hated this about the scala collections API. mapValues is lazy but everything else isn’t? :’(

This revision was automatically updated to reflect the committed changes.