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

Remove integer-gmp / ghc-bignum dependency #371

Merged
merged 3 commits into from
Mar 1, 2021

Conversation

Bodigrim
Copy link
Contributor

...and associated flags/CPP.

@sjakobi
Copy link
Member

sjakobi commented Feb 26, 2021

Wow, what a nice simplification! :)

How does it affect performance?

@Bodigrim
Copy link
Contributor Author

Replacing quotRemInteger by quotRem and unboxed tuples by boxed does not incur a measurable performance regression. I believe GHC optimizes this stuff under the hood, it is very simple worker-wrapper transformation, and anyways at this point division costs trump everything else.

Surprisingly, toIntegralSized appeared to hit performance on short Integer by ~25%. I replaced it with an old-school fromInteger / toInteger, and now performance is back to baseline.

Copy link
Member

@sjakobi sjakobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprisingly, toIntegralSized appeared to hit performance on short Integer by ~25%. I replaced it with an old-school fromInteger / toInteger, and now performance is back to baseline.

I think that might be worth reporting as a perf bug.

Data/ByteString/Builder/ASCII.hs Show resolved Hide resolved
@Bodigrim
Copy link
Contributor Author

@sjakobi Does removing a flag constitute a breaking change? Shall I retain integer-simple flag (doing nothing) for 0.11 branch?

@sjakobi
Copy link
Member

sjakobi commented Feb 28, 2021

@sjakobi Does removing a flag constitute a breaking change? Shall I retain integer-simple flag (doing nothing) for 0.11 branch?

Good question! My guess would be that Cabal flags are "part of the API" in a way, and therefore changing them is a breaking change. I've opened haskell/pvp#33 to be sure.

Maybe wait with the backport for now…

@hsyl20
Copy link
Contributor

hsyl20 commented Mar 1, 2021

Does removing a flag constitute a breaking change? Shall I retain integer-simple flag (doing nothing) for 0.11 branch?

This change should only impact GHC afaict and we can easily drop this flag from GHC's build system when we will bump the bytestring submodule. I would just remove it.

@sjakobi
Copy link
Member

sjakobi commented Mar 1, 2021

I'm convinced. Let's not treat this as a breaking change.

@Bodigrim Bodigrim added this to the 0.11.2.0 milestone Mar 1, 2021
@Bodigrim Bodigrim merged commit b639596 into haskell:master Mar 1, 2021
@Bodigrim Bodigrim deleted the no-integer-gmp branch March 1, 2021 18:24
noughtmare pushed a commit to noughtmare/bytestring that referenced this pull request Dec 12, 2021
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.

None yet

3 participants