Akka + type tags = memory leaks

[Eugene Burmako]

I just got a report from our early adopter, Nikita Volkov.

Nikita's found a memory leak in reflection [1]. He says that Akka has
methods that have TypeTag context bounds, and these type tags
indirectly use a non-weak cache in internal.Types. Bummer.

Therefore I have several questions:
1) What's the use case for type tags in Akka? Do you guys have any
stress tests to expose these kinds of glitches?
2) What other non-weak caches do we have in reflect.internal? Anything
off the top of your head?

[1] https://dl.dropbox.com/u/10497693/memory-profile.png
[2] https://dl.dropbox.com/u/10497693/nonweak-cache-in-types.png

[Nikita Volkov]

Correction. Eugene got me wrong. It wasn't Akka that utilized the TypeTag, but my code inside the "receive" method implementation of Akka actor. Here's a memory snapshot produced with YourKit from which the pictures posted by Eugene were taken from (~200M to download, 2G decompressed): https://dl.dropbox.com/u/34793919/snapshot.7z.

четверг, 20 сентября 2012 г., 22:18:43 UTC+4 пользователь Eugene Burmako написал:

[Eugene Burmako]

My bad, sorry for the fuss...

This still leaves reflect.internal questions. Why is the uniques cache not weak? What other non weak caches do we have?

[Josh Suereth]

So, first things first

(1) I'd open a bug on this.  

(2)  We need to find a way to test memory usage in partest, if possible, so we can try to prevent issues like these.   Does anyone have something like this besides iulian's checker for the IDE?

(3) We isolate the problem area, using #2 if we can, and fix it.

I'd say that memory leaks, even for the "experimental" reflection are blockers.

- Josh

[√iktor Ҡlang]

On Thu, Sep 20, 2012 at 9:20 PM, Josh Suereth <joshua....@gmail.com> wrote:

So, first things first

(1) I'd open a bug on this.  

(2)  We need to find a way to test memory usage in partest, if possible, so we can try to prevent issues like these.   Does anyone have something like this besides iulian's checker for the IDE?

(3) We isolate the problem area, using #2 if we can, and fix it.

I'd say that memory leaks, even for the "experimental" reflection are blockers.

+BigInt.PositiveInfinity

 

- Josh

On Thu, Sep 20, 2012 at 2:37 PM, Eugene Burmako <eugene....@epfl.ch> wrote:

My bad, sorry for the fuss...

This still leaves reflect.internal questions. Why is the uniques cache not weak? What other non weak caches do we have?

On 20 Sep 2012, at 20:30, Nikita Volkov <nikita....@gmail.com> wrote:

Correction. Eugene got me wrong. It wasn't Akka that utilized the TypeTag, but my code inside the "receive" method implementation of Akka actor. Here's a memory snapshot produced with YourKit from which the pictures posted by Eugene were taken from (~200M to download, 2G decompressed): https://dl.dropbox.com/u/34793919/snapshot.7z.

четверг, 20 сентября 2012 г., 22:18:43 UTC+4 пользователь Eugene Burmako написал:

I just got a report from our early adopter, Nikita Volkov.

Nikita's found a memory leak in reflection [1]. He says that Akka has
methods that have TypeTag context bounds, and these type tags
indirectly use a non-weak cache in internal.Types. Bummer.

Therefore I have several questions:
1) What's the use case for type tags in Akka? Do you guys have any
stress tests to expose these kinds of glitches?
2) What other non-weak caches do we have in reflect.internal? Anything
off the top of your head?

[1] https://dl.dropbox.com/u/10497693/memory-profile.png
[2] https://dl.dropbox.com/u/10497693/nonweak-cache-in-types.png

--
Viktor Klang

Akka Tech Lead

Typesafe - The software stack for applications that scale

Twitter: @viktorklang

[Paul Phillips]

On Thu, Sep 20, 2012 at 11:37 AM, Eugene Burmako <eugene....@epfl.ch> wrote:

This still leaves reflect.internal questions. Why is the uniques cache not weak?

I expect the sounds of the compiler exploding would be audible for miles.  You could try it I guess.

More likely improvement may be found in being more particular about what constitutes a unique type.  This has been on my list for pretty much ever.  A huge percentage of the types in uniques are refinement types which typically have no utility beyond the calculation (lub, usually) being done at that moment.

What other non weak caches do we have?

uniques dominates all other caches, weak or otherwise.

[Paul Phillips]

On Thu, Sep 20, 2012 at 12:20 PM, Josh Suereth <joshua....@gmail.com> wrote:

(1) I'd open a bug on this.  

(2)  We need to find a way to test memory usage in partest, if possible, so we can try to prevent issues like these.   Does anyone have something like this besides iulian's checker for the IDE?

(3) We isolate the problem area, using #2 if we can, and fix it.

I had completely forgotten I wrote this until I stumbled across it yesterday.  Forgot I wrote it twice, in fact - first in scala, then in java so I could actually compare different scala versions without lots of binary composting.

  https://github.com/paulp/jvm-memory-tracker

[Josh Suereth]

Think there's a way we can modify that (in an automated fashion) to detect memory leaks?   I can't see anything other than "Don't use more than X MB for this test" being a reasonable test.   However, if that sounds reasonable, I can jump on trying to get something into partest that can track/fail on memory usage with a java-agent fork of this one.

Personally, I really want something that can do that for testing Collections and catching memory usage issues....

- Josh

[Paul Phillips]

On Thu, Sep 20, 2012 at 12:40 PM, Josh Suereth <joshua....@gmail.com> wrote:

Think there's a way we can modify that (in an automated fashion) to detect memory leaks?   I can't see anything other than "Don't use more than X MB for this test" being a reasonable test.   However, if that sounds reasonable, I can jump on trying to get something into partest that can track/fail on memory usage with a java-agent fork of this one.

Don't use more than X MB for this test should work, and even be semi-stable across different test machines and environments (unlike, say, "don't use more than X seconds.") But a true memory leak should be unambiguous.  If you can keep doing a thing which should be constant memory, either it is and the utilization is flat, or it isn't and you can watch it creep upward to OOM.  I'm not sure what we're thinking of as a leak when it comes to uniques, because it is cleared after every compiler run...

Speaking of uniques, I'd forgotten about this atrocity.  This really must go.  As usual, I complained at the time.

  object ConstantType extends ConstantTypeExtractor {

    def apply(value: Constant): ConstantType = {

      val tpe = new UniqueConstantType(value)

      if (value.tag == ClazzTag) {

        // if we carry a classOf, we might be in trouble

        // http://groups.google.com/group/scala-internals/browse_thread/thread/45185b341aeb6a30

        // I don't have time for a thorough fix, so I put a hacky workaround here

        val alreadyThere = uniques findEntry tpe

        if ((alreadyThere ne null) && (alreadyThere ne tpe) && (alreadyThere.toString != tpe.toString)) {

          // we need to remove a stale type that has the same hashcode as we do

          // HashSet doesn't support removal, and this makes our task non-trivial

          // also we cannot simply recreate it, because that'd skew hashcodes (that change over time, omg!)

          // the only solution I can see is getting into the underlying array and sneakily manipulating it

          val ftable = uniques.getClass.getDeclaredFields().find(f => f.getName endsWith "table").get

          ftable.setAccessible(true)

          val table = ftable.get(uniques).asInstanceOf[Array[AnyRef]]

          def overwrite(hc: Int, x: Type) {

            def index(x: Int): Int = math.abs(x % table.length)

            var h = index(hc)

            var entry = table(h)

            while (entry ne null) {

              if (x == entry)

                table(h) = x

              h = index(h + 1)

              entry = table(h)

            }

          }

          overwrite(tpe.##, tpe)

        }

      }

      unique(tpe).asInstanceOf[ConstantType]

    }

  }

[Eugene Burmako]

Nikita, could you please elaborate on the scenario you used to leak
the stuff? Did you generate some new code? Invoke toolboxes? Or just
repeatedly reflected over the same set of classes?

[Nikita Volkov]

I did use toolboxes to generate several new classes, but not repeatedly - once generated they got cached. And I did reflect over the same set of classes repeatedly which included these generated classes too.

2012/9/21 Eugene Burmako <eugene....@epfl.ch>

[Eugene Burmako]

Here's the list of top-level vals/vars/objects declared in
scala.reflect.internals and scala.reflect.runtime: https://gist.github.com/3760764.

Of those we have four things that are supposedly leaking:
1) Symbols.originalOwner (easy to fix => just don't track original
owners in reflection, because they are only used in GenASM/GenJVM)
2) Types.lubResults/glbResults (these are cleaned up by public lub/glb
functions, but there are non-public overloads of lub/glb, which don't
clean up the cache and are called by public stuff like <:< and
baseClasses)
3) Types.undoLog (only cleaned up in typer cleanup, which never
happens in reflection)
4) Types.uniques (we already discussed this one)

Toolboxes are currently leaking a bit. Most caches are cleaned up
correctly, except for perRunCaches used in typeCheck. perRunCaches are
only flushed in Global.compileUnits, which isn't called by typeCheck.
That's a minor problem I think, since we can just call
perRunCaches.clear in typeCheck.

WDYT?

[Ismael Juma]

On Thu, Sep 20, 2012 at 8:46 PM, Paul Phillips <pa...@improving.org> wrote:

Speaking of uniques, I'd forgotten about this atrocity.  This really must go.  As usual, I complained at the time.

Wow, that is pretty bad.

Best,

Ismael

[Eugene Burmako]

Could you share the sources?

On Sep 21, 1:31 pm, Nikita Volkov <nikita.y.vol...@gmail.com> wrote:
> I did use toolboxes to generate several new classes, but not repeatedly -
> once generated they got cached. And I did reflect over the same set of
> classes repeatedly which included these generated classes too.
>

> 2012/9/21 Eugene Burmako <eugene.burm...@epfl.ch>

[Eugene Burmako]

Please let's not turn this thread into yet another "wtf all the codez
are broken" flame war.

[Paul Phillips]

It's all well and good to take the high ground, but it rings hollow. Why not just fix the code?

Sent from my iPhone

[Paul Phillips]

On Fri, Sep 21, 2012 at 4:38 AM, Eugene Burmako <eugene....@epfl.ch> wrote:

Please let's not turn this thread into yet another "wtf all the codez
are broken" flame war.

Here are some more points I would like to make based on this response.

Preface: if anyone gets it in their head to place me under moderation again, please go all the way and remove me from the scala lists.  I'm not someone who will work under threat of suppression.

It seems the major lesson derived from my plea a couple weeks ago was "paulp should be less of a jerk." If any regular committer noticed anything else at all, anything to do with the meaning of the words, they kept it to themselves.

This response is yet another variation on that response, which is to avoid the subject being raised at all costs in favor of saddling up the high horse and citing passages from robert's rules of order.

I do not know what else to say.  My participation is hanging by a thread.  Someone had better start responding to *what I am saying* and not to *how I am saying it* or I am going to find something else to do.

[√iktor Ҡlang]

If there is such pieces of code that are compromising the platform (like the one shown above), I suggest that there should be blocker-tickets opened to fix/rewrite such sections. Then there simply cannot be any release until they are rectified. 

my 2¢

Cheers,

√

[Eugene Burmako]

Paul, I wasn't replying to you and didn't even think about that.

Here's what I really think. Thanks for reminding me about the ConstantType issue. There was an associated bug, but I never got time to fix it. Today I have time, and (speaking of high ground and hollowing ringings) I'm in the middle of the fix. If you didn't raise this issue, I would most likely only fix it after 2.10.0, therefore your message in this thread was definitely useful. And I don't have any intention of criticizing useful comments.

If you want me to go as far as proclaiming my attitude towards you, then let it be so. I'm not embarassed to publicly say that you're great, that I learned a lot from you, and that I hope to learn even more.

[Paul Phillips]

On Fri, Sep 21, 2012 at 7:08 AM, Eugene Burmako <eugene....@epfl.ch> wrote:

Here's what I really think. Thanks for reminding me about the ConstantType issue. There was an associated bug, but I never got time to fix it. Today I have time, and (speaking of high ground and hollowing ringings) I'm in the middle of the fix.

Great.

Understand that today's message was driven 95% by the fact that nobody acknowledged my last one except to hold it against me; yours only lit that fuse.