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

Starting offset not necessary #174

Closed
ekmett opened this issue Jun 5, 2019 · 2 comments · Fixed by #175
Closed

Starting offset not necessary #174

ekmett opened this issue Jun 5, 2019 · 2 comments · Fixed by #175
Milestone

Comments

@ekmett
Copy link
Member

ekmett commented Jun 5, 2019

GHC >= 8.2 exports plusForeignPtr, which allows you to switch the address targeted by a ForeignPtr. The finalizers remain attached. However, this appears to be implementable, by hand, using the public contents of GHC.ForeignPtr clear back to at least GHC 7.0!

Removing the offset would save an entire word per bytestring!

It may be worth including a backwards compatibility shim on modern enough GHCs to support bidirectional pattern synonyms. This would enable most consumers that use the Data.ByteString.Internal module to proceed unpatched.

e.g. something like

module Data.ByteString.Internal 
  ( ByteString(BS, PS) -- add some c preprocessing to handle older GHCs that don't support data-type associated patterns
  , ...
  ) where

...

data ByteString = BS !(ForeignPtr Word8) !Int

pattern PS :: ForeignPtr Word -> Int -> Int -> ByteString
pattern PS fp 0 len <- BS fp len where
  PS fp o len = BS (plusForeignPtr fp o) len

With sufficient rewrite rules for replacing plusForeignPtr fp 0 with fp it should give almost all of the benefits of a complete rewrite of user code.

This would be a major version bump as the behavior of round-tripping through the pattern synonym would change.

I don't have any compilers older than GHC 7.0 to test with, so I'm not sure how much further back into antiquity this can be ported. The current lower bound on base for bytestring is set at GHC 6.12, so this doesn't appear to slide our compatibility window much if at all.

@ekmett
Copy link
Member Author

ekmett commented Jun 5, 2019

Update: I'm implementing this idea in

https://github.com/ekmett/bytestring/tree/no-offset

ekmett added a commit to ekmett/bytestring that referenced this issue Jun 5, 2019
@ekmett
Copy link
Member Author

ekmett commented Jun 6, 2019

Addressed by #175.

@hvr hvr added the blocked: next-major-version-bump blocked until a major version bump is happening label Dec 19, 2019
vdukhovni pushed a commit to vdukhovni/bytestring that referenced this issue May 27, 2020
vdukhovni pushed a commit to vdukhovni/bytestring that referenced this issue May 27, 2020
vdukhovni pushed a commit to vdukhovni/bytestring that referenced this issue May 29, 2020
vdukhovni pushed a commit to vdukhovni/bytestring that referenced this issue May 29, 2020
vdukhovni pushed a commit to vdukhovni/bytestring that referenced this issue Jun 3, 2020
vdukhovni pushed a commit to vdukhovni/bytestring that referenced this issue Jun 3, 2020
@sjakobi sjakobi linked a pull request Jun 25, 2020 that will close this issue
@Bodigrim Bodigrim removed the blocked: next-major-version-bump blocked until a major version bump is happening label Sep 10, 2020
@Bodigrim Bodigrim added this to the 0.11.0.0 milestone Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants