Skip to content

Use Set.alterF in nubOrd#1228

Merged
meooow25 merged 1 commit into
haskell:masterfrom
meooow25:nubord-alterf
Jul 23, 2026
Merged

Use Set.alterF in nubOrd#1228
meooow25 merged 1 commit into
haskell:masterfrom
meooow25:nubord-alterf

Conversation

@meooow25

@meooow25 meooow25 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Compared to the previous implementation of member followed by insert, alterF uses memberIndex and insertAt. This avoids all the element comparisons in insert, at the cost of memberIndex being a little more expensive than member. I expect this change to be an improvement in typical use cases.

Related: #1205


Benchmarks with GHC 9.14:

Name                                Time - - - - - - - -    Allocated - - - - -
                                         A       B     %         A       B     %
nubOrd.issue1202_distinct            48 μs   46 μs   -4%    672 KB  672 KB   +0%
nubOrd.issue1202_repeat             3.8 μs  4.0 μs   +3%     16 KB   16 KB   +0%
nubOrd.noFusion_distinct             44 μs   43 μs   -1%    633 KB  633 KB   +0%
nubOrd.noFusion_repeat              1.5 μs  1.9 μs  +19%    152 B   152 B    +0%
nubOrd.strings_noFusion_distinct    166 μs  112 μs  -32%    611 KB  626 KB   +2%
nubOrd.strings_noFusion_repeat      3.6 μs  4.2 μs  +16%    136 B   152 B   +11%

@sjakobi sjakobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's very good to reduce the number of Ord comparisons, but it would be better to if the new implementation didn't allocate all these Maybes.

@meooow25

Copy link
Copy Markdown
Contributor Author

Ah but it doesn't, the Maybes disappear after optimizations. You'll notice that the allocations numbers haven't increased (except for nubOrd.strings_noFusion_repeat, I haven't checked what's up with that but the Core looks good so I am satisfied).

Compared to the previous implementation of `member` followed by
`insert`, `alterF` uses `memberIndex` and `insertAt`. This avoids all
the element comparisons in `insert`, at the cost of `memberIndex` being
a little more expensive than `member`. I expect this change to be an
improvement in typical use cases.
@meooow25 meooow25 changed the title Use alterF in nubOrd Use Set.alterF in nubOrd Jul 23, 2026
@meooow25
meooow25 merged commit 4af9d12 into haskell:master Jul 23, 2026
19 checks passed
@meooow25
meooow25 deleted the nubord-alterf branch July 23, 2026 18:55
@sjakobi

sjakobi commented Jul 24, 2026

Copy link
Copy Markdown
Member

Ah but it doesn't, the Maybes disappear after optimizations. You'll notice that the allocations numbers haven't increased (except for nubOrd.strings_noFusion_repeat, I haven't checked what's up with that but the Core looks good so I am satisfied).

Ah, I missed that! And it appears that you managed to reduce the allocations in #1229! 👍

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.

2 participants