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

Mark nub and sort functions as INLINE #51

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Conversation

toyboot4e
Copy link
Contributor

Fixes #50. Notes:

  • nubByMut is marked as INLINABLE. Change it to INLINE if it's faster in some case.
  • sortUniq is marked as INLINE, but without any benchmark. I guess it's ok as it's very small.

@erikd
Copy link
Owner

erikd commented Feb 2, 2025

Something weird with cabal and tests. I have fixed this on master. Please rebase and force push.

`nubMutBy` is marked as `INLINABLE`, but change it if `INLINE` is
faster
@toyboot4e
Copy link
Contributor Author

I've done rebase and force push. Btw the 七味唐がらし made me smile ha ha.

@toyboot4e
Copy link
Contributor Author

Simplifier ticks exhausted

Sounds like too much inlining, as functions called from sort are also inlined?

@erikd
Copy link
Owner

erikd commented Feb 3, 2025

Btw the 七味唐がらし made me smile ha ha.

Where? I have not even seen it. Was not me.

@toyboot4e
Copy link
Contributor Author

toyboot4e commented Feb 3, 2025

Ha ha I saw it in your profile picture (👉 🍶).

About this PR,

  • The nub family change would be ok.
  • The sort family change can be reverted if you don't like it (as the test fails for ghc-8.0.2).
    • I guess they should rather change the internal implementation from INLINE to INLINABLE + stToPrim, however, it would be another benchmark and a PR.

@erikd
Copy link
Owner

erikd commented Feb 3, 2025

Ha ha I saw it in your profile picture

Ah didn't even know what it was called. I just know I like it on eg Teriaki Chciken.

@erikd
Copy link
Owner

erikd commented Feb 4, 2025

I will merge this as is and remove the ghc-8.0 CI job.

@erikd erikd merged commit 3229aae into erikd:master Feb 5, 2025
12 of 13 checks passed
@erikd
Copy link
Owner

erikd commented Feb 5, 2025

Version 0.9.1.0 uploaded to Hackage.

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.

sort and nub functions should be marked as {-# INLINE #-}
2 participants