@gregrow Thanks for taking the time to look at this.

Given you have a proposed solution, can you comment on why the stack overflow occurs and how the change addresses this? Is there test code that demonstrates the issue and demonstrates the proposal addresses the issue?

Right, Mark, I should be more verbose.

Let's have a loop:

TreeMap<Integer, Integer> tm = TreeMap.treeMap(Ord.comparableOrd(),
  List.single(P.p(0, 0)));
while (true) {
   tm = tm.update(0, a -> a+1)._2();
}

The loop is supposed to be infinite. However, it throws StackOverflowError after a number of iterations in the current version of functionaljava - If one uses -Xss228k JVM option, it only takes 8318 iterations to get there (on openjdk-1.8.0.31 x86_64).

The reason I found is that the following transformation in fj.data.TreeMap.update(final K k, final F<V, V> f):

P2.<K, Option<V>, Option<V>>map2_(Option.<V, V>map().f(f))

returns a function that creates a closure over its argument.

And so, iterative update at a key in a TreeMap effects in a chain of closures being stored in a map entry.

This chain must be fully traversed each time an entry is accessed (even when accessed implicitly e.g. when it's just compared during a search in a baking set). It impacts performance, may cause stack overflows and grows heap consumption.

The fix I propose is to evaluate the transformation result (the closure) down to a value object before storing it in a baking set.

I'm not sure if

compose(P2.tuple(P.p2()), ...)

is the best way to achieve it in functionaljava. But I think you won't hesitate to refactor if needed.

I see now. In the code P2.<K, Option<V>, Option<V>>map2_(Option.<V, V>map().f(f)), map2_ calls P2.map2 which maps over the second item in the tuple. It does this lazily, referring to the old tuple for the methods _1() and _2(). Each update then builds up this sequence of lazy calls which then eventually overflows the stack. Your solution forces the value out of the lazy tuple and then puts it back into a new tuple.

I had a play with some test code and saw the overflow and how the change fixes the situation (which then obviously runs faster).