Scala Programming Language
  1. Scala Programming Language
  2. SI-1419

don't generate unneeded references to outer objects in inner classes

    Details

      Description

      Currently (change set 16246 of trunk) named inner classes always contain a reference to their enclosing object, regardless of whether it is use or not. This is similar to the old behavior of anonymous inner classes as demonstrated in https://lampsvn.epfl.ch/trac/scala/ticket/854 . I think named inner classes should not retain a reference to the enclosing object if it is not required so that subtle memory leaks can be avoided.

        Issue Links

          Activity

          Hide
          Erik Engbrecht added a comment -

          Replying to [comment:5 stepancheg]:
          Where is "someValue" declared? I can't tell when it would be GC'd without knowing where the references to it are.

          That being said, isn't the point of a WeakHashMap to allow objects to be GC'd?

          Show
          Erik Engbrecht added a comment - Replying to [comment:5 stepancheg] : Where is "someValue" declared? I can't tell when it would be GC'd without knowing where the references to it are. That being said, isn't the point of a WeakHashMap to allow objects to be GC'd?
          Hide
          David R. MacIver added a comment -

          Don't be ridiculous. If something doesn't use something else, the compiler shouldn't insert a reference to it. That's a large part of the point of having lexical scoping. Code which relies on undocumented behaviour about garbage collecting outer classes is inherently broken, and code which puts classes in the place they logically belong shouldn't have to face a penalty because the compiler generates non-optimal code.

          Show
          David R. MacIver added a comment - Don't be ridiculous. If something doesn't use something else, the compiler shouldn't insert a reference to it. That's a large part of the point of having lexical scoping. Code which relies on undocumented behaviour about garbage collecting outer classes is inherently broken, and code which puts classes in the place they logically belong shouldn't have to face a penalty because the compiler generates non-optimal code.
          Hide
          Oscar Boykin added a comment -

          This issue hits scalding ( https://github.com/twitter/scalding ) because when we serialize inner classes on hadoop, they are pulling in references to the outer class, even if it is not used.

          It makes generic serialization libraries (like Kryo, which we use) very difficult because we have to see if the outer$ parameter is really accessed or not (something Spark works around with ASM which is obviously an awful hack).

          We see this issue on scala 2.9.2.

          Our main work around now is to tell our users "don't do that". But it's difficult to explain to non-experts why it should matter.

          Show
          Oscar Boykin added a comment - This issue hits scalding ( https://github.com/twitter/scalding ) because when we serialize inner classes on hadoop, they are pulling in references to the outer class, even if it is not used. It makes generic serialization libraries (like Kryo, which we use) very difficult because we have to see if the outer$ parameter is really accessed or not (something Spark works around with ASM which is obviously an awful hack). We see this issue on scala 2.9.2. Our main work around now is to tell our users "don't do that". But it's difficult to explain to non-experts why it should matter.
          Hide
          Adriaan Moors added a comment -

          we hope to tackle this for 2.11 aka the performance edition,
          it's too big a change for 2.10.X

          Show
          Adriaan Moors added a comment - we hope to tackle this for 2.11 aka the performance edition, it's too big a change for 2.10.X
          Hide
          Adriaan Moors added a comment -

          Unassigning and rescheduling to M6 as previous deadline was missed.

          Show
          Adriaan Moors added a comment - Unassigning and rescheduling to M6 as previous deadline was missed.

            People

            • Assignee:
              Unassigned
              Reporter:
              Erik Engbrecht
              TracCC:
              Erik Engbrecht, Seth Tisue
            • Votes:
              8 Vote for this issue
              Watchers:
              16 Start watching this issue

              Dates

              • Created:
                Updated:

                Development