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.

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

Improving efficiency and compatability of traverse implementation #221

Merged
merged 1 commit into from
Oct 9, 2018

Conversation

recursion-ninja
Copy link
Contributor

This pull request is a partial fix for #220. It addresses an incompatibility defect between vector and ghc-compact.

The new traverse implementation is strictly more efficient and more compatible.

@recursion-ninja
Copy link
Contributor Author

Is there a time line for merging this? I'd like it to be able to use the Traversable instance of Vector in conjunction with Compact Regions as soon as possible. Is there anything else required for this to be merged?

@cartazio
Copy link
Contributor

cartazio commented Oct 9, 2018 via email

@cartazio cartazio merged commit 12c608d into haskell:master Oct 9, 2018
@cartazio
Copy link
Contributor

cartazio commented Oct 9, 2018

ok, this looks like a good fix for me, i've merged it into master,
i'll see about doing a bug fix release for that later this week
(got a bit of travel next few days)

Q: do we fundamentally need the from/to lists here? I feel like there must be some clever way to do stream fusion here, though i suppose that would mess with applicative not being strict ..

@recursion-ninja
Copy link
Contributor Author

@cartazio I can't comment on if there is a more clever way to implement traverse to invoke better stream fusion. I don't know the internals of the library well enough. I just know that calls to fromList will cause run-time exceptions when the resulting vector is used in a compact region, so I replaced fromList with fromListN.

I look forward to the bug fix release, hopefully this week!

@recursion-ninja
Copy link
Contributor Author

@cartazio any ETA on the bug fix version being uploaded to hackage?

@cartazio
Copy link
Contributor

derp, got a bit buried with some travel then being ill for a span, putting it back on top of the queue

cartazio pushed a commit that referenced this pull request Nov 14, 2018
…oxed vectors. (gh pr #221, fixes bug #220)

previously traverse used unknown size fromList rather fromListN for constructing Boxed vectors.
In the presence of compact regions the implementation strategy for fromList results in program crashes. Now traverse on Boxed vectors uses the input vector size for constructing the result vector.
@recursion-ninja
Copy link
Contributor Author

Hey, can we get a new version of vector uploaded to hackage with this improvement.

@cartazio
Copy link
Contributor

cartazio commented Dec 7, 2018 via email

@cartazio
Copy link
Contributor

cartazio commented Dec 7, 2018 via email

@cartazio
Copy link
Contributor

cartazio commented Dec 7, 2018

https://hackage.haskell.org/package/vector-0.12.0.2/candidate is the candidate
please confirm it fixes your issue :)

@cartazio
Copy link
Contributor

cartazio commented Dec 7, 2018

once i get https://travis-ci.org/haskell/vector/builds/465105124 green i'll do the release

@cartazio
Copy link
Contributor

cartazio commented Dec 7, 2018

released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants