Move IsStream type class definition out of the low level StreamK module #1223
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
adithyaov
reviewed
Sep 10, 2021
| -- continuation and a yield continuation. | ||
| {-# INLINE_EARLY mkStream #-} | ||
| mkStream :: IsStream t | ||
| => (forall r. State K.Stream m a |
There was a problem hiding this comment.
I remember there being an issue or a comment about fixing K.Stream in State. No comment. Just stating it here.
adithyaov
reviewed
Sep 10, 2021
| {-# INLINE_LATE drain #-} | ||
| drain :: Monad m => Stream m a -> m () | ||
| -- drain = foldrM (\_ xs -> xs) (return ()) | ||
| drain (Stream step state) = go SPEC state |
There was a problem hiding this comment.
It is required in low level modules like instances of SerialT and other types.
adithyaov
reviewed
Sep 10, 2021
| @@ -471,7 +480,7 @@ sourceConcatMapId val n = | |||
| {-# INLINE concatMapBySerial #-} | |||
| concatMapBySerial :: Int -> Int -> Int -> IO () | |||
| concatMapBySerial outer inner n = | |||
| S.drain $ S.concatMapBy S.serial | |||
| S.drain $ S.concatMapWith S.serial | |||
There was a problem hiding this comment.
Are concatMapWith and concatMapBy same?
There was a problem hiding this comment.
concatMapBy is older name, later renamed to concatMapWith.
adithyaov
approved these changes
Sep 10, 2021
|
|
harendra-kumar
force-pushed
the
streamly-core
branch
from
80465a3 to
89ac933
Compare
adithyaov
reviewed
Sep 12, 2021
bin/test.sh
Outdated
| --cabal-build-options) shift; CABAL_BUILD_OPTIONS+=$1; shift ;; | ||
| --hpc-report-options) shift; HPC_REPORT_OPTIONS=$1; shift ;; | ||
| --rtsopts) shift; RTS_OPTIONS=$1; shift ;; | ||
| --cabal-build-options) shift; CABAL_BUILD_OPTIONS+=" $1"; shift ;; |
There was a problem hiding this comment.
I fixed this in another change. I'll undo that.
|
I am going to squash and merge it. |
harendra-kumar
force-pushed
the
streamly-core
branch
from
89ac933 to
60dd74e
Compare
The IsStream type class contains MonadAsync type in the consM method. This refactor allows lifting MonadAsync, therefore, dependency on monad-control out of the low level modules.
To include tests that require large amounts of RAM
harendra-kumar
force-pushed
the
streamly-core
branch
from
60dd74e to
f6b88dc
Compare
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.
Description
Move
IsStreamdefinition out of the low levelStreamKmodule and push it up in a higher level IsStream/Type module. This is a step towards creating a streamly-core package devoid of any non-boot library dependencies, as mentioned in #533 . Also, this will facilitate moving to monomorphic stream types and modules in future, currently we use polymorphic streams based on IsStream type class.The IsStream type class contains MonadAsync type in the consM method. This refactor allows lifting MonadAsync, therefore, dependency on monad-control out of the low level modules.
The main changes are in StreamK/Type.hs, StreamK.hs, IsStream/Type.hs, Stream/Serial.hs, the rest of the changes are mostly a fallout of these changes. Need to review these files and a few other files with important changes, the rest of the changes can be glanced over.
Things to check
Performance Notes
The
foldrSbenchmark has regressed by around 100 times, but the original perf numbers were good because ofa trivial foldr/build fusion. If we see
foldrSMapwhich actually performs a mapusing
foldrSthen it has improved by 2x.iterateStatebenchmark now requires a SPECIALIZE pragma to specialize.Other degradations, but the core looks the same for all these cases, may want to verify once again:
Other than these there are no significant regressions, there are many improvements though.