Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplified function memoization #1837

Merged
merged 1 commit into from
Jan 26, 2017
Merged

Simplified function memoization #1837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 4 additions & 24 deletions javaslang/generator/Generator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1639,36 +1639,16 @@ def generateMainClasses(): Unit = {
} else {
${val mappingFunction = (checked, i) match {
case (true, 0) => s"() -> $Try.of(this::apply).get()"
case (true, 1) => s"t -> $Try.of(() -> this.apply(t)).get()"
case (true, _) => s"t -> $Try.of(() -> tupled.apply(t)).get()"
case (true, _) => s"t -> $Try.of(() -> apply($params)).get()"
case (false, 0) => s"this::apply"
case (false, 1) => s"this"
case (false, _) => s"tupled"
}
val forNull = (checked, i) match {
case (true, 1) => s"$Try.of(() -> apply(null))::get"
case (false, 1) => s"() -> apply(null)"
case _ => null
case (false, _) => s"tupled()"
}
if (i == 0) xs"""
return ($className$fullGenerics & Memoized) Lazy.of($mappingFunction)::get;
""" else if (i == 1) xs"""
final Object lock = new Object();
final ${im.getType("java.util.Map")}<$generics, R> cache = new ${im.getType("java.util.HashMap")}<>();
return ($className$fullGenerics & Memoized) t1 -> {
synchronized (lock) {
return cache.computeIfAbsent(t1, $mappingFunction);
}
};
""" else xs"""
final Object lock = new Object();
final ${im.getType("java.util.Map")}<Tuple$i<$generics>, R> cache = new ${im.getType("java.util.HashMap")}<>();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new WeakHashMap<>(); to avoid memory leaks?
@danieldietrich, @ruslansennov, @zsolt-donca?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new WeakHashMap<>(); to avoid memory leaks?

What about this scenario?

Function1<Tuple2<Integer, Integer>, Tuple2<Integer, Integer>> f;
Function1<Tuple2<Integer, Integer>, Tuple2<Integer, Integer>> mem = f.memoized();
Tuple2<Integer, Integer> keyInstance = Tuple.of(1, 2);
Tuple2<Integer, Integer> result = mem.apply(keyInstance);

        //
        //  hard work

keyInstance = null;

        ///  hard work
        ///

Tuple2<Integer, Integer> newInstance = Tuple.of(1, 2);
Tuple2<Integer, Integer> result2 = mem.apply(newInstance); // calculation again (?)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's a concern, as weak references can be eliminated at any time. The problem is even more problematic for FunctionN where N ≥ 2, as the key (tuple) is built internally, immediately discarding any other reference to it.

The above problem with the GC prematurely dropping the data could be eliminated by using soft references; for details, see this:

SoftReferences aren't required to behave any differently than WeakReferences, but in practice softly reachable objects are generally retained as long as memory is in plentiful supply. This makes them an excellent foundation for a cache, such as the image cache described above, since you can let the garbage collector worry about both how reachable the objects are (a strongly reachable object will never be removed from the cache) and how badly it needs the memory they are consuming.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paplorinc @zsolt-donca I follow @ruslansennov that the function should not compute a value twice (imagine Math::random). Not sure how we can ensure that a memoized function in GC'ed if not referenced any more but one of its cached values is still in use 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think memory out of bounds exception is worse, but I'm ok with both :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will merge this version in. Memory leaks may also appear in other cases, e.g. when capturing the outer object in a lambda. Optimizations need to be done carefully. Actually we have a working version, which is great. Thx!

final ${checked.gen("Checked")}Function1<Tuple$i<$generics>, R> tupled = tupled();
return ($className$fullGenerics & Memoized) ($params) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of($params), $mappingFunction);
}
};
return ($className$fullGenerics & Memoized) ($params)
-> Memoized.of(cache, Tuple.of($params), $mappingFunction);
"""
}
}
Expand Down
10 changes: 3 additions & 7 deletions javaslang/src-gen/main/java/javaslang/CheckedFunction1.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,9 @@ default CheckedFunction1<T1, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<T1, R> cache = new HashMap<>();
return (CheckedFunction1<T1, R> & Memoized) t1 -> {
synchronized (lock) {
return cache.computeIfAbsent(t1, t -> Try.of(() -> this.apply(t)).get());
}
};
final Map<Tuple1<T1>, R> cache = new HashMap<>();
return (CheckedFunction1<T1, R> & Memoized) (t1)
-> Memoized.of(cache, Tuple.of(t1), t -> Try.of(() -> apply(t1)).get());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/CheckedFunction2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,9 @@ default CheckedFunction2<T1, T2, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple2<T1, T2>, R> cache = new HashMap<>();
final CheckedFunction1<Tuple2<T1, T2>, R> tupled = tupled();
return (CheckedFunction2<T1, T2, R> & Memoized) (t1, t2) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2), t -> Try.of(() -> tupled.apply(t)).get());
}
};
return (CheckedFunction2<T1, T2, R> & Memoized) (t1, t2)
-> Memoized.of(cache, Tuple.of(t1, t2), t -> Try.of(() -> apply(t1, t2)).get());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/CheckedFunction3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,14 +192,9 @@ default CheckedFunction3<T1, T2, T3, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple3<T1, T2, T3>, R> cache = new HashMap<>();
final CheckedFunction1<Tuple3<T1, T2, T3>, R> tupled = tupled();
return (CheckedFunction3<T1, T2, T3, R> & Memoized) (t1, t2, t3) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3), t -> Try.of(() -> tupled.apply(t)).get());
}
};
return (CheckedFunction3<T1, T2, T3, R> & Memoized) (t1, t2, t3)
-> Memoized.of(cache, Tuple.of(t1, t2, t3), t -> Try.of(() -> apply(t1, t2, t3)).get());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/CheckedFunction4.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,9 @@ default CheckedFunction4<T1, T2, T3, T4, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple4<T1, T2, T3, T4>, R> cache = new HashMap<>();
final CheckedFunction1<Tuple4<T1, T2, T3, T4>, R> tupled = tupled();
return (CheckedFunction4<T1, T2, T3, T4, R> & Memoized) (t1, t2, t3, t4) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3, t4), t -> Try.of(() -> tupled.apply(t)).get());
}
};
return (CheckedFunction4<T1, T2, T3, T4, R> & Memoized) (t1, t2, t3, t4)
-> Memoized.of(cache, Tuple.of(t1, t2, t3, t4), t -> Try.of(() -> apply(t1, t2, t3, t4)).get());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/CheckedFunction5.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,9 @@ default CheckedFunction5<T1, T2, T3, T4, T5, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple5<T1, T2, T3, T4, T5>, R> cache = new HashMap<>();
final CheckedFunction1<Tuple5<T1, T2, T3, T4, T5>, R> tupled = tupled();
return (CheckedFunction5<T1, T2, T3, T4, T5, R> & Memoized) (t1, t2, t3, t4, t5) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3, t4, t5), t -> Try.of(() -> tupled.apply(t)).get());
}
};
return (CheckedFunction5<T1, T2, T3, T4, T5, R> & Memoized) (t1, t2, t3, t4, t5)
-> Memoized.of(cache, Tuple.of(t1, t2, t3, t4, t5), t -> Try.of(() -> apply(t1, t2, t3, t4, t5)).get());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/CheckedFunction6.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,14 +252,9 @@ default CheckedFunction6<T1, T2, T3, T4, T5, T6, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple6<T1, T2, T3, T4, T5, T6>, R> cache = new HashMap<>();
final CheckedFunction1<Tuple6<T1, T2, T3, T4, T5, T6>, R> tupled = tupled();
return (CheckedFunction6<T1, T2, T3, T4, T5, T6, R> & Memoized) (t1, t2, t3, t4, t5, t6) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3, t4, t5, t6), t -> Try.of(() -> tupled.apply(t)).get());
}
};
return (CheckedFunction6<T1, T2, T3, T4, T5, T6, R> & Memoized) (t1, t2, t3, t4, t5, t6)
-> Memoized.of(cache, Tuple.of(t1, t2, t3, t4, t5, t6), t -> Try.of(() -> apply(t1, t2, t3, t4, t5, t6)).get());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/CheckedFunction7.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,9 @@ default CheckedFunction7<T1, T2, T3, T4, T5, T6, T7, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple7<T1, T2, T3, T4, T5, T6, T7>, R> cache = new HashMap<>();
final CheckedFunction1<Tuple7<T1, T2, T3, T4, T5, T6, T7>, R> tupled = tupled();
return (CheckedFunction7<T1, T2, T3, T4, T5, T6, T7, R> & Memoized) (t1, t2, t3, t4, t5, t6, t7) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3, t4, t5, t6, t7), t -> Try.of(() -> tupled.apply(t)).get());
}
};
return (CheckedFunction7<T1, T2, T3, T4, T5, T6, T7, R> & Memoized) (t1, t2, t3, t4, t5, t6, t7)
-> Memoized.of(cache, Tuple.of(t1, t2, t3, t4, t5, t6, t7), t -> Try.of(() -> apply(t1, t2, t3, t4, t5, t6, t7)).get());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/CheckedFunction8.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,14 +297,9 @@ default CheckedFunction8<T1, T2, T3, T4, T5, T6, T7, T8, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple8<T1, T2, T3, T4, T5, T6, T7, T8>, R> cache = new HashMap<>();
final CheckedFunction1<Tuple8<T1, T2, T3, T4, T5, T6, T7, T8>, R> tupled = tupled();
return (CheckedFunction8<T1, T2, T3, T4, T5, T6, T7, T8, R> & Memoized) (t1, t2, t3, t4, t5, t6, t7, t8) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3, t4, t5, t6, t7, t8), t -> Try.of(() -> tupled.apply(t)).get());
}
};
return (CheckedFunction8<T1, T2, T3, T4, T5, T6, T7, T8, R> & Memoized) (t1, t2, t3, t4, t5, t6, t7, t8)
-> Memoized.of(cache, Tuple.of(t1, t2, t3, t4, t5, t6, t7, t8), t -> Try.of(() -> apply(t1, t2, t3, t4, t5, t6, t7, t8)).get());
}
}

Expand Down
10 changes: 3 additions & 7 deletions javaslang/src-gen/main/java/javaslang/Function1.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,9 @@ default Function1<T1, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<T1, R> cache = new HashMap<>();
return (Function1<T1, R> & Memoized) t1 -> {
synchronized (lock) {
return cache.computeIfAbsent(t1, this);
}
};
final Map<Tuple1<T1>, R> cache = new HashMap<>();
return (Function1<T1, R> & Memoized) (t1)
-> Memoized.of(cache, Tuple.of(t1), tupled());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/Function2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,9 @@ default Function2<T1, T2, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple2<T1, T2>, R> cache = new HashMap<>();
final Function1<Tuple2<T1, T2>, R> tupled = tupled();
return (Function2<T1, T2, R> & Memoized) (t1, t2) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2), tupled);
}
};
return (Function2<T1, T2, R> & Memoized) (t1, t2)
-> Memoized.of(cache, Tuple.of(t1, t2), tupled());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/Function3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,14 +192,9 @@ default Function3<T1, T2, T3, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple3<T1, T2, T3>, R> cache = new HashMap<>();
final Function1<Tuple3<T1, T2, T3>, R> tupled = tupled();
return (Function3<T1, T2, T3, R> & Memoized) (t1, t2, t3) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3), tupled);
}
};
return (Function3<T1, T2, T3, R> & Memoized) (t1, t2, t3)
-> Memoized.of(cache, Tuple.of(t1, t2, t3), tupled());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/Function4.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,9 @@ default Function4<T1, T2, T3, T4, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple4<T1, T2, T3, T4>, R> cache = new HashMap<>();
final Function1<Tuple4<T1, T2, T3, T4>, R> tupled = tupled();
return (Function4<T1, T2, T3, T4, R> & Memoized) (t1, t2, t3, t4) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3, t4), tupled);
}
};
return (Function4<T1, T2, T3, T4, R> & Memoized) (t1, t2, t3, t4)
-> Memoized.of(cache, Tuple.of(t1, t2, t3, t4), tupled());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/Function5.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,9 @@ default Function5<T1, T2, T3, T4, T5, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple5<T1, T2, T3, T4, T5>, R> cache = new HashMap<>();
final Function1<Tuple5<T1, T2, T3, T4, T5>, R> tupled = tupled();
return (Function5<T1, T2, T3, T4, T5, R> & Memoized) (t1, t2, t3, t4, t5) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3, t4, t5), tupled);
}
};
return (Function5<T1, T2, T3, T4, T5, R> & Memoized) (t1, t2, t3, t4, t5)
-> Memoized.of(cache, Tuple.of(t1, t2, t3, t4, t5), tupled());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/Function6.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,14 +252,9 @@ default Function6<T1, T2, T3, T4, T5, T6, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple6<T1, T2, T3, T4, T5, T6>, R> cache = new HashMap<>();
final Function1<Tuple6<T1, T2, T3, T4, T5, T6>, R> tupled = tupled();
return (Function6<T1, T2, T3, T4, T5, T6, R> & Memoized) (t1, t2, t3, t4, t5, t6) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3, t4, t5, t6), tupled);
}
};
return (Function6<T1, T2, T3, T4, T5, T6, R> & Memoized) (t1, t2, t3, t4, t5, t6)
-> Memoized.of(cache, Tuple.of(t1, t2, t3, t4, t5, t6), tupled());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/Function7.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,9 @@ default Function7<T1, T2, T3, T4, T5, T6, T7, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple7<T1, T2, T3, T4, T5, T6, T7>, R> cache = new HashMap<>();
final Function1<Tuple7<T1, T2, T3, T4, T5, T6, T7>, R> tupled = tupled();
return (Function7<T1, T2, T3, T4, T5, T6, T7, R> & Memoized) (t1, t2, t3, t4, t5, t6, t7) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3, t4, t5, t6, t7), tupled);
}
};
return (Function7<T1, T2, T3, T4, T5, T6, T7, R> & Memoized) (t1, t2, t3, t4, t5, t6, t7)
-> Memoized.of(cache, Tuple.of(t1, t2, t3, t4, t5, t6, t7), tupled());
}
}

Expand Down
9 changes: 2 additions & 7 deletions javaslang/src-gen/main/java/javaslang/Function8.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,14 +297,9 @@ default Function8<T1, T2, T3, T4, T5, T6, T7, T8, R> memoized() {
if (isMemoized()) {
return this;
} else {
final Object lock = new Object();
final Map<Tuple8<T1, T2, T3, T4, T5, T6, T7, T8>, R> cache = new HashMap<>();
final Function1<Tuple8<T1, T2, T3, T4, T5, T6, T7, T8>, R> tupled = tupled();
return (Function8<T1, T2, T3, T4, T5, T6, T7, T8, R> & Memoized) (t1, t2, t3, t4, t5, t6, t7, t8) -> {
synchronized (lock) {
return cache.computeIfAbsent(Tuple.of(t1, t2, t3, t4, t5, t6, t7, t8), tupled);
}
};
return (Function8<T1, T2, T3, T4, T5, T6, T7, T8, R> & Memoized) (t1, t2, t3, t4, t5, t6, t7, t8)
-> Memoized.of(cache, Tuple.of(t1, t2, t3, t4, t5, t6, t7, t8), tupled());
}
}

Expand Down
13 changes: 13 additions & 0 deletions javaslang/src/main/java/javaslang/λ.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package javaslang;

import java.io.Serializable;
import java.util.Map;

/**
* This is a general definition of a (checked/unchecked) function of unknown parameters and a return type R.
Expand Down Expand Up @@ -74,5 +75,17 @@ default boolean isMemoized() {
* Zero Abstract Method (ZAM) interface for marking functions as memoized using intersection types.
*/
interface Memoized {
static <T extends Tuple, R> R of(Map<T, R> cache, T key, Function1<T, R> tupled) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good solution, we can do that. I currently have not a better solution for the synching problem. It is hard to test.

I'm not that familiar with proper cache invalidation (Weak/Soft-Refs) but we need to keep the tuples in the cache as long as the memoized function is referenced.

synchronized (cache) {
if (cache.containsKey(key)) {
return cache.get(key);
} else {
final R value = tupled.apply(key);
cache.put(key, value);
return value;
}
}
}

}
}