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

Avoid boxing arrays #230

Closed

Conversation

treeowl
Copy link
Collaborator

@treeowl treeowl commented Feb 14, 2019

Previously, we used a lot of things that looked like

runST (act >>= unsafeFreeze)

The trouble is that GHC can't unbox past the runRW# primitive
that runST is based on. So this actually allocates an Array
constructor which we will then throw away immediately.

The way to avoid this is to call runRW# manually to produce
a raw SmallArray#, then apply the Array constructor on the
outside.

@treeowl
Copy link
Collaborator Author

treeowl commented Feb 14, 2019

@Fuuzetsu, @m-renaud, I don't trust my ability to benchmark. Do you think one of you could try this out? My own benchmarks suggest that it's generally good, sometimes neutral, and (for reasons that are entirely mysterious to me) fairly bad for the fromList/long/String benchmark.

@treeowl treeowl force-pushed the unbox-arrays branch 2 times, most recently from 59d81ac to 725c929 Compare February 14, 2019 06:16
Previously, we used a lot of things that looked like

    runST (act >>= unsafeFreeze)

The trouble is that GHC can't unbox past the `runRW#` primitive
that `runST` is based on. So this actually allocates an `Array`
constructor which we will then throw away immediately.

The way to avoid this is to call `runRW#` manually to produce
a raw `SmallArray#`, then apply the `Array` constructor on the
outside.
@m-renaud
Copy link
Collaborator

I'll be out of town until Tuesday next week, but I can take a look when I get back.

@treeowl
Copy link
Collaborator Author

treeowl commented Jun 4, 2020

We need to see if this is still necessary on GHC HEAD. @bgamari just recently implemented and merged a GHC patch to try to fix this sort of thing.

@sjakobi
Copy link
Member

sjakobi commented Jun 18, 2020

We need to see if this is still necessary on GHC HEAD. @bgamari just recently implemented and merged a GHC patch to try to fix this sort of thing.

I'll mark this PR as a draft then to signal that it's not currently waiting for a review.

@sjakobi sjakobi marked this pull request as draft June 18, 2020 01:38
@sjakobi
Copy link
Member

sjakobi commented Mar 3, 2022

I have rebased this PR on my sjakobi/unbox-arrays branch.

I'll take a look at the perf implications once GHC 9.2.2 is released.

We need to see if this is still necessary on GHC HEAD. @bgamari just recently implemented and merged a GHC patch to try to fix this sort of thing.

@treeowl In which GHC version was this work released?

@treeowl
Copy link
Collaborator Author

treeowl commented Mar 3, 2022

I don't remember. And the new stuff sometimes just defeats what we want to do. So everything needs to be pored over.

@sjakobi
Copy link
Member

sjakobi commented Mar 12, 2022

I have opened #375 to finish this.

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

Successfully merging this pull request may close these issues.

3 participants