Visitar URL original
Tweak `insertKeyExists` and `deleteKeyExists` by sjakobi · Pull Request #458 · haskell-unordered-containers/unordered-containers · GitHub
Skip to content

Tweak insertKeyExists and deleteKeyExists - #458

Merged
sjakobi merged 8 commits into
masterfrom
sjakobi/keyExists-tweaks
May 10, 2022
Merged

sjakobi merged 8 commits into
masterfrom
sjakobi/keyExists-tweaks

Conversation

@sjakobi

@sjakobi sjakobi commented May 7, 2022

Copy link
Copy Markdown
Member

(This currently includes #456)

Comment thread Data/HashMap/Internal.hs Resolved
Comment thread Data/HashMap/Internal.hs Resolved
Comment thread Data/HashMap/Internal.hs Resolved
Comment thread Data/HashMap/Internal.hs Resolved
@sjakobi

sjakobi commented May 7, 2022

Copy link
Copy Markdown
Member Author

These new invariant tests are great! :)

      alterF
        valid:                             FAIL
          *** Failed! Falsified (after 28 tests and 26 shrinks):
          {_->[Nothing]}
          K {hash = 9, _x = A}
          fromList [(K {hash = 0, _x = A},1),(K {hash = 9, _x = B},1),(K {hash = 9, _x = A},1)]
          [Invalid (INV6_misplaced_hash 0) (SubHashPath {partialHash = 9, lengthInBits = 5})] /= [Valid]
          Use --quickcheck-replay=980663 to reproduce.
          Use -p '/Data.HashMap.Strict.alterF.valid/' to rerun this test only.

@sjakobi

sjakobi commented May 8, 2022 •

Copy link
Copy Markdown
Member Author

Note for myself:

I somehow couldn't reproduce the bugs above with cabal test -O0. I should look into this!

EDIT: Explained by @treeowl in #459.

@treeowl

treeowl commented May 9, 2022

Copy link
Copy Markdown
Collaborator

Oh, I'm starting to understand now. You're shifting as you go to avoid an extra argument?

@sjakobi

sjakobi commented May 9, 2022

Copy link
Copy Markdown
Member Author

Oh, I'm starting to understand now. You're shifting as you go to avoid an extra argument?

Exactly.

@sjakobi

sjakobi commented May 10, 2022

Copy link
Copy Markdown
Member Author

It seems that right now, there are no benchmarks using this code. All benchmarks for alterF seem to rely on rules that rewrite the code to either insert or delete.

This may change with #404 though.

@sjakobi
sjakobi force-pushed the sjakobi/keyExists-tweaks branch from 64fbd45 to 7e813f2 Compare May 10, 2022 01:37
@sjakobi

sjakobi commented May 10, 2022

Copy link
Copy Markdown
Member Author

I can't find a significant speedup in the alter benchmarks, but not a slowdown either. The generated Core seems slightly better.

@sjakobi
sjakobi marked this pull request as ready for review May 10, 2022 02:09
@sjakobi
sjakobi merged commit 8c20f7a into master May 10, 2022
@sjakobi
sjakobi deleted the sjakobi/keyExists-tweaks branch May 10, 2022 19:00
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