Iterator groupby #84
Iterator groupby #84
Conversation
|
Benchmark results:
I was honestly expecting a slightly more drastic difference, although it's perhaps worth noting that the implementation labelled "current" is what's in master right now, not what's in the most recently released version. However, the version of |
| . (forall h. Eff (st :: ST h | r) (f (STArray h))) | ||
| -> Eff r (f Array) | ||
| runSTArray' = | ||
| unsafeCoerce |
There was a problem hiding this comment.
I'm not convinced it's safe to just coerce here, without copying the arrays. For example, what about something like:
runSTArray do
xs <- initSomeSTArray
setTimeout 1000 (modifySomehow xs)
pure xsThe return value is supposed to be a regular array, but it will get modified in place one second after the function returns.
There was a problem hiding this comment.
Oh, good point. I guess the current runSTArray function has the same issue too? What if we restrict the row type to a closed row?
There was a problem hiding this comment.
Yes, I think restricting to a closed row should solve the problem.
There was a problem hiding this comment.
Cool, thanks. In that case I think I will publish a minor release before doing that, since this will be a breaking change.
For instances where you want to return more than just one STArray from an STArray computation. For example, a pair of STArrays (with potentially different types), or an STArray of STArrays, or even an STArray of nonempty STArrays. The possibilities are endless! Also use unsafe-coerce instead of FFI.
Implement `groupBy` using `Data.Array.ST.Iterator`; this should make it faster.
\x y -> odd x && odd y is not an equivalence relation! It doesn't satisfy reflexivity.
hdgarrood
force-pushed
the
iterator-groupby
branch
from
8a653c4 to
6c57fc9
Compare
hdgarrood
force-pushed
the
iterator-groupby
branch
from
6c57fc9 to
162f43a
Compare
|
I decided to change |
|
Thanks again, and sorry for the delay! |
|
Hmm, I just remembered this is a breaking change. What shall we do here? Revert, or make a major release? |
|
No problem! Up to you whether you think it's worth merging now and making a major release. We could also maybe make a branch off of master from before any breaking changes were merged in case we ever want to do a minor or patch release before the next major one? |
|
I think we have a few libraries which need major releases:
Any others? |
|
I'm not aware of any off the top of my head |
I recommend reviewing these commits one at a time, as they are each independent of each other. The
runSTArray'commit is more or less what I was describing in #78.