Remove invalid optimization from put that negatively impacts performance #86
Conversation
Increases flag memory usage by 300%, while significantly streamlining logic and improving performance
|
Do you mean "decreases flag memory usage"? |
|
The first commit -- 9b12aa4 -- was the only one I intended to get captured in the pull request, in that case removing an unnecessary extra conditional that adds overhead (but was seemingly added as an optimization), discovered during some quick analysis regarding the performance of C in the nucleotide benchmark game. It is a change that can only be positive, even for projects that might have poorly considered dependencies on internals. The second commit -- 348601f -- is a potentially breaking change that reduces the amount of bit packing, increasing the memory footprint for flags (albeit still a tiny overhead for all but the most enormous of hash tables), however it has a dramatic benefit to performance on all architectures because it removes three expensive bit shifts per loop evaluation. It can break uses that might have a direct hard-fixed dependencies on the structure of flags, for instance (though any that used the macros provided would be fine), so it was more an example of cost-benefit analysis of memory versus performance. |
|
Both of these patches are great additions |
kh_put_##name has an early check for an initial empty bucket that negatively impacts significant use, while only benefiting unrealistically simplistic benchmarks (e.g. inserting a single item).
As an example, for k-nucleotide, with this revision on an arbitrary test machine the entire process completes in ~3.3s real time, with ~9.5s of user time. Prior to this revision it completes in ~3.9s, with ~11.0s of user time.