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.

Sign up for GitHub

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

Jump to bottom

Prefer Data.HashSet.member to Data.Foldable.elem #2886

Merged
merged 3 commits into from
May 2, 2022
Merged

Prefer Data.HashSet.member to Data.Foldable.elem #2886

merged 3 commits into from
May 2, 2022

Conversation

sergv
Copy link
Contributor

@sergv sergv commented May 1, 2022

No description provided.

fendor
fendor approved these changes May 2, 2022
View reviewed changes
Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@pepeiborra
Copy link
Collaborator

What is the motivation to prefer HashSet.member over Data.Foldable.elem?

@jhrcek
Copy link
Collaborator

jhrcek commented May 2, 2022 •
edited
Loading

I suspect it's because HashSet.member is O(log n) whereas Data.Foldable.elem needs to traverse the whole structure (at best linear in number of elements). For example relude even uses type errors to warn you about using elem with sets: https://hackage.haskell.org/package/relude-1.0.0.1/docs/Relude-Foldable-Fold.html#v:elem

@pepeiborra
Copy link
Collaborator

I suspect it's because HashSet.member is O(log n) whereas Data.Foldable.elem needs to traverse the whole structure (at best linear in number of elements). For example relude even uses type errors to warn you about using elem with sets: https://hackage.haskell.org/package/relude-1.0.0.1/docs/Relude-Foldable-Fold.html#v:elem

Fair enough, elem lacks Hashable powers so it cannot do anything smart

@pepeiborra pepeiborra enabled auto-merge (squash) May 2, 2022 09:32
@pepeiborra pepeiborra merged commit fe573f8 into haskell:master May 2, 2022
@sergv
Copy link
Contributor Author

sergv commented May 2, 2022

Indeed, elem is linear since it must work with only Eq constraint.

sloorush pushed a commit to sloorush/haskell-language-server that referenced this pull request May 21, 2022
@sergv @pepeiborra @sloorush
Prefer Data.HashSet.member to Data.Foldable.elem (haskell#2886)
7842379 
* Prefer Data.HashSet.member to Data.Foldable.elem

* Remove unused binding

Co-authored-by: Pepe Iborra <pepeiborra@gmail.com>
hololeap pushed a commit to hololeap/haskell-language-server that referenced this pull request Aug 26, 2022
@sergv @pepeiborra @hololeap
Prefer Data.HashSet.member to Data.Foldable.elem (haskell#2886)
a24b0b1 
* Prefer Data.HashSet.member to Data.Foldable.elem

* Remove unused binding

Co-authored-by: Pepe Iborra <pepeiborra@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fendor fendor fendor approved these changes

@OliverMadine OliverMadine Awaiting requested review from OliverMadine

@pepeiborra pepeiborra Awaiting requested review from pepeiborra

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sergv @pepeiborra @jhrcek @fendor