-
Notifications
You must be signed in to change notification settings - Fork 186
Build Set and Map more efficiently and consistently #1042
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
Conversation
|
Hmm.. the failing strictness test is a bit silly. It is defeated by the new fusible definition. Anyway, it gets removed in #1021. |
|
How do things go when fusion doesn't happen? |
|
Is there any way to get side by side comparisons of times and allocations? All the befores followed by all the afters is hard to look at. |
c234f10 to
b7d639b
Compare
Seems to be around the same, perhaps slightly slower. You can see these in the table when there is no
Right, I had a script for that. Updated the benchmarks section. |
|
Another benefit of this implementation (which I did not consider before, #351) is that it will allow defining functions like Going further, we could expose the Builder stuff if there is a demand for it. |
b7d639b to
4c05817
Compare
|
Rebased and updated benchmarks. If there are no concerns, I would like to merge this soon, so that I can consider this |
Use "Builder"s to implement some Set and Map construction functions.
As a result, some have become good consumers in terms of list fusion,
and all are now O(n) for non-decreasing input.
Fusible Fusible O(n) for O(n) for
before after before after
Set.fromList No Yes Strict incr Non-decr
Set.map - - Strict incr Non-decr
Map.fromList No Yes Strict incr Non-decr
Map.fromListWith Yes Yes Never Non-decr
Map.fromListWithKey Yes Yes Never Non-decr
Map.mapKeys - - Strict incr Non-decr
Map.mapKeysWith - - Never Non-decr
4c05817 to
1e55239
Compare
5b436ba to
b8f6016
Compare
Use "Builder"s to implement some Set and Map construction functions.
As a result, some have become good consumers in terms of list fusion,
and all are now O(n) for non-decreasing input.
For #1027. Also related to #949.
Benchmarks with GHC 9.10.1:
Set:
Map:
Note: You may notice desc fusion cases haven't improved, though they really should. It turns out to be a
enumFromThen/fusion issue, and not really a problem with our code. I opened a GHC issue for it: GHC#25259