-
Notifications
You must be signed in to change notification settings - Fork 178
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
Build Set and Map more efficiently and consistently #1042
base: master
Are you sure you want to change the base?
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. |
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
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. |
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 on GHC 9.6.3:
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