score:3

Accepted answer

the problem is that the anonymous function passed to mapcont closes over next. in turn, this is closed-over by the lazy variable we pass to enumiterator, which is closed-over by the new enumerator formed by enumiterator1, which is closed-over by the anonymous function in apply, which is finally closed-over by the anonymous function passed to mapcont for the next iteration.

so, by a chain of closures, each enumerator closes over its predecessor. this would probably happen whether next was captured or not, so you'd have a minor memory leak either way. however, you end up capturing next in one of these closures, which means that every value generated by your iterator stays in memory until the whole process is complete (and these values take up a lot of memory).

by moving next inside the anonymous function passed to mapcont, next is not captured in our chain of closures any more, so the main memory leak disappears (although your closures still close over each other, which may be a concern).

the best way to fix this is probably to simplify it. as brian kernighan famously said:

everyone knows that debugging is twice as hard as writing a program in the first place. so if you're as clever as you can be when you write it, how will you ever debug it?

i'm not certain i fully understand the code, but i suspect the following is equivalent:

def enumiterator1[e, f[_]: monad](x: => iterator[e]) : enumeratort[e, f] =
  new enumeratort[e, f] {
    def apply[a] = {
      val xs = x
      def innerapply(s: stept[e, f, a]): iterateet[e, f, a] = {
        if(xs.isempty) s.pointi
        else {
          val next = xs.next
          s mapcont { cont => // renamed k to cont, as the function, rather than the variable, is k
            cont(iteratee.elinput(next)) >>== innerapply
          }
        }
      }
      innerapply
    }
  }

you might also benefit from making things more explicit. for example, what if rather than having an anonymous enumeratort that implicitly closes over anything it needs within its scope, you define a named class, with top level scope, and pass in anything it needs explicitly.

i used -xx:+heapdumponoutofmemoryerror, visualvm, and javap to find the cause of the issue. they should be everything you need.

update

i think i'm starting to grok what the code's supposed to do, and i've updated my code accordingly. i think the problem was the use of enumiterator1[e, f](xs).apply[a]. the code was creating a new enumeratort just to get at its apply method, but creating a by-name variable and closing over everything-and-its-dog in the process. since the value of xs doesn't change from one recursion to the next, we create an innerapply method which closes over the val xs, and re-use innerapply.

update 2

i was curious, so i had a look around in the scalaz source to see how they solve this problem. here's some code with a similar bent from scalaz itself:

def enumiterator[e, f[_]](x: => iterator[e])(implicit mo: monadpartialorder[f, io]) : enumeratort[e, f] =
  new enumeratort[e, f] {
    import mo._ // remove this line, and you can copy and paste it into your code
    def apply[a] = {
      def go(xs: iterator[e])(s: stept[e, f, a]): iterateet[e, f, a] =
        if(xs.isempty) s.pointi
        else {
          s mapcont { k => 
            val next = xs.next
            k(elinput(next)) >>== go(xs)
          }
        }
      go(x)
    }
  }

they use currying, rather than closure, to capture xs, but it's still an "inner apply" approach.


Related Query

More Query from same tag