Fix eager execution due to flatMap #5430
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
flyingsilverfin
requested review from
haikalpribadi and
lolski
as code owners
haikalpribadi
approved these changes
Sep 17, 2019
|
@flyingsilverfin LGTM! don't forget to set the appropriate labels and milestone next time ;) |
flyingsilverfin
added a commit
that referenced
this pull request
Sep 26, 2019
## What is the goal of this PR? Replace io.vavr (#5430) that was introduced to solve the lack of lazy `flatMap` that is mentioned in #5430. We then determined that conversion to the `io.vavr` Stream implementation is more lazy, but the conversion step itself does a single call to `.next()`. In practical terms, this means Grakn always precomputed the first answer on query, which is still not fully lazy. This led to the decision to restore prior behavior and solve the eagerness problem by implementing our own lazy stream merging solution. ## What are the changes implemented in this PR? * Implement a lazy stream merging solution that uses `stream.iterator()` conversions interally * Remove `io.vavr` as a dependency * tests for lazy stream merging
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the goal of this PR?
We currently have a use of
flatMap()on the stream of answers that is produced from Janus via our various transformations to produce Concepts and Answers. TheflatMap()at the end of this sequence of stream manipulations is not necessarily lazy.See:
https://bugs.openjdk.java.net/browse/JDK-8075939
To fix this we introduced
Vavrwhich is able to properly handle lazy streams andflatMapoperations. We introduce a minimal change using this library as required here, converting to a Vavr stream and back once the flatMap is added to the stream.What are the changes implemented in this PR?
io.vavr.vavrthat has no transitive dependencies beyond one of its own further packagesflatMapinQueryExecutor.matchwith a few steps to utilisevavr's stream implementation that actually lazy