INLINABLE insert and delete, bench, small doc additions.#19
INLINABLE insert and delete, bench, small doc additions.#19robinp wants to merge 7 commits intolarskuhtz:masterfrom
Conversation
So callsite can specialize or inline them as it wills. Some noticable imprevement on benchmarks: ``` insert: 16ms -> 11ms delete: 26ms -> 16ms ``` Also added the splitCuckooFilter function so benchmark can start from a pre-filled CF. Could have made it that either it only returns a new CF, or returns a pair whose fst is the original CF (with the same ByteArray) but with the split gen.. but those leave too much opportunity for accidental misuse.
larskuhtz
left a comment
larskuhtz
left a comment
There was a problem hiding this comment.
Thanks a lot for this PR. The perf improvements look really nice.
I added a couple suggestions in the review. I also like to understand why the split is needed in the delete benchmark.
| deleteBench label n cf0 xs = bench (label <> " - " <> show n) $ whnfIO f | ||
| where | ||
| f = do | ||
| (cf, _) <- splitCuckooFilter cf0 |
There was a problem hiding this comment.
What is the purpose of splitting the filter here before doing the deletes?
There was a problem hiding this comment.
The idea was to pre-create the filter once, and then the benchmark would only time the deletions (not the filter creation). The split was just a way to prevent the original filter from being mutated across the different bench iterations.
But maybe there are other ways to get this, without split - I learned there is perRunEnv in criterion that lets one create the input separately from the measurement. That would let one get rid of the split functionality.
I'm open to whichever way is preferred, or alternatives.
| -- Note: if the potentially inserted elements have many repetitions, and you | ||
| -- don't intend to delete items, rather just use the filter as a set, then it is | ||
| -- advised to perform a 'member' check before insert. Otherwise repeated items | ||
| -- will take up slots and cause high load or even premature insertion failure. | ||
| -- |
Co-authored-by: Lars Kuhtz <lakuhtz@gmail.com>
Co-authored-by: Lars Kuhtz <lakuhtz@gmail.com>
|
@larskuhtz thank you for the comments! Oops, sorry for the random typo. |
2 participants
Added benchmarks for the main public interface ops, also marked insert and delete INLINABLE, so the callsite can optimize further. This had some small but noticable impact on the benchmark.