Memoization seems broken on Java 9

[l0rinc]

See: #1563 (comment)

[danieldietrich]

Note: Currently it is not clear if it is a bug in Javaslang or JDK9. But it seems to work correct with JDK8. However, Javaslang 2.1.0 needs to work correct with a future version of the JDK. So we need to solve this for the upcoming 2.1.0 release.

[l0rinc]

The problematic test case is:

private static final CheckedFunction5<Integer, Integer, Integer, Integer, Integer, Integer> recurrent1 = (i1, i2, i3, i4, i5) -> i1 <= 0 ? i1 : CheckedFunction5Test.recurrent2.apply(i1 - 1, i2, i3, i4, i5) + 1;
private static final CheckedFunction5<Integer, Integer, Integer, Integer, Integer, Integer> recurrent2 = CheckedFunction5Test.recurrent1.memoized();

@Test
public void shouldCalculatedRecursively() throws Throwable {
    assertThat(recurrent1.apply(11, 11, 11, 11, 11)).isEqualTo(11);
    assertThat(recurrent1.apply(22, 22, 22, 22, 22)).isEqualTo(22);
}

which does seem broken, i.e. it should modify the cache, while it's already modifying it :/.
Do we really need this functionality?

[l0rinc]

The tests fail forEuler 67 also:

private final static Function3<Vector<Vector<Integer>>, Integer, Integer, Integer> smart = Function3.of(
        (Vector<Vector<Integer>> tr, Integer row, Integer col) -> {
            int value = tr.get(row).get(col);
            if (row == tr.length() - 1) {
                return tr.get(row).get(col);
            } else {
                return value + Math.max(Euler67Test.smart.apply(tr, row + 1, col), Euler67Test.smart.apply(tr, row + 1, col + 1));
            }
        }
).memoized();

since it calls itself also.

---

I think therefore that memoization was working incorrectly before also (i.e. calculating things twice), but in Java 9 they seem to have added a validation check to avoid this.

Will provide a PR to ask for comments.

[l0rinc]

@danieldietrich:
It seems to me that we cannot easily support recursive memoization: http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-February/031237.html

[danieldietrich]

@paplorinc Thank you, that link is interesting!

[danieldietrich]

@paplorinc Did your PR #1837 made memoization Java 9 compatible? Internally we do not use recursive memoization any more, right?

[danieldietrich]

@paplorinc ping... (see #1762 (comment))

[l0rinc]

Yes, all the tests passed on JVM 9 (though the build fails because of the Scala code generation).☺️

[danieldietrich]

@paplorinc Many thanks! 😊 The Scala code generator issue isn't critical for now because we build with Java 8 and will not move Javaslang to the Java 9 code level. Java 10 will be a very interesting release for us!