Make memoization compatible with MPP #331
Conversation
Add some docs and an example
…ore into ab/make-memoization-mpp
…ore into ab/make-memoization-mpp
| * ``` | ||
| * | ||
| * Note that calling this function with the same parameters in parallel might cause the function to be executed twice. | ||
| */ | ||
| fun <R> (() -> R).memoize(): () -> R = object : () -> R { |
There was a problem hiding this comment.
Does it make sense to define them as suspended functions. Given the nature of memoizing, or am I mistaken here?
cc @nomisRev
There was a problem hiding this comment.
This PR just updates the existing code to remove the dependency on Java classes. We can do a variant of this for suspending code as well.
Kinda silly that this needs to be defined separately tho, since normal code should work for suspending code. Only the other way around is not true.
There was a problem hiding this comment.
Actually, the suspend version is the only pure/referential transparent implementation since it involves allocation mutable shared state to do the memoisation.
There was a problem hiding this comment.
Yes, that was one of my concerns.
Maybe it's worth deprecating it first and rolling out a suspended version either here or in Arrow Fx
|
@rachelcarmena @nomisRev Seems this PR is failing for arrow-fx due to test flakiness(?) or might have my changes affected that? |
|
@aballano seems to be the case of something flaky in Arrow Fx and Arrow Fx RxJava which are both deprecated so not very concerning but I've never seen the flaky test in Arrow Fx RxJava. I restarted the build. Have you seen this multiple times on this PR? |
|
@nomisRev Yes, the previous build after merging master also failed due to failed test for Bracket in the stream package |
Replace ConcurrentHashMap impl by atomic to be able to work in MPP