fuzz: Cleanup muhash fuzz target #22942

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2109-fuzzMuhashCleanup changing 1 files +34 −39

MarcoFalke commented at 4:08 PM on September 10, 2021: member

No description provided.
MarcoFalke added the label Refactoring on Sep 10, 2021
MarcoFalke added the label Tests on Sep 10, 2021
MarcoFalke requested review from fjahr on Sep 10, 2021

in src/test/fuzz/muhash.cpp:33 in fa12ae5211 outdated

  70 | +            muhash.Finalize(out);
  71 | +            CallOneOf(
  72 | +                fuzzed_data_provider,
  73 | +                [&] {
  74 | +                    MuHash3072 muhash_inverse_order;
  75 | +                    muhash_inverse_order = MuHash3072();

fjahr commented at 10:35 PM on September 14, 2021:

I think this line is redundant.

MarcoFalke commented at 8:37 AM on September 22, 2021:

restored original code

fjahr commented at 12:24 AM on September 15, 2021: member

Concept ACK

I think this can be simplified further to use only one CallOneOf but still achieve the same thing: https://github.com/fjahr/bitcoin/commit/2ff513b51bbb870611a7be3ac640507ade299370. I also added some more comments there and used a constant for the initial state hash which should help the speed some more.
MarcoFalke force-pushed on Sep 22, 2021
MarcoFalke commented at 8:37 AM on September 22, 2021: member

I think this can be simplified further to use only one CallOneOf but still achieve the same thing: fjahr@2ff513b. I also added some more comments there and used a constant for the initial state hash which should help the speed some more.

Thanks for the 2am review and code suggestion. Took ~your branch and squashed into one commit.

fuzz: Cleanup muhash fuzz target

Can be reviewed with -W --ignore-all-space

Fixes:
* Calling ConsumeRandomLengthByteVector 4 times, when 2 is enough.
* Slow execution speed: Finalize is expensive because it invokes
  division. Speed up the target by calling Finalize() at most twice per
  fuzz input.

0000dca6f0

MarcoFalke force-pushed on Sep 22, 2021
MarcoFalke commented at 8:48 AM on September 22, 2021: member

Speedup: Locally it now takes 29% of the original time
MarcoFalke commented at 12:41 PM on September 22, 2021: member

CI Before: 2654 s https://cirrus-ci.com/task/4629294711308288?logs=ci#L4695

CI After: 1011 s https://cirrus-ci.com/task/6682329813876736?logs=ci#L4189
fjahr commented at 10:14 PM on September 27, 2021: member

ACK 0000dca6f0e4dda212bf8adf555b68f2c7464ff8
MarcoFalke merged this on Sep 28, 2021
MarcoFalke closed this on Sep 28, 2021
MarcoFalke deleted the branch on Sep 28, 2021
sidhujag referenced this in commit c75cf55406 on Sep 28, 2021
DrahtBot locked this on Oct 30, 2022

Contributors

MarcoFalke

fjahr

Labels

Refactoring Tests