fuzz: speed up addrman

[brunoerg]

There is no need to iterate 10'000 times and
call Add 10'000 times at once (we only add
1000 addresses at once in practice). These values
were introduced in 214d905.

I see an avg of 40 exec/s on master for this target and 115 exec/s on this branch.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/30688.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	Eunovo

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

[maflcko]

we only add
1000 addresses at once in practice

Are you referring to MAX_ADDR_TO_SEND? If yes, it could make sense to clarify this in the description or even in a code comment, given that the variable name seems to indicate it is only used for sending, not for processing/adding.

[brunoerg]

Are you referring to MAX_ADDR_TO_SEND? If yes, it could make sense to clarify this in the description or even in a code comment, given that the variable name seems to indicate it is only used for sending, not for processing/adding.

Yes, the name is a little bit confusing since it's the maximum number of addresses permitted in an ADDR message. So we use it for sending and receiving.

if (vAddr.size() > MAX_ADDR_TO_SEND)
{
    Misbehaving(*peer, strprintf("%s message size = %u", msg_type, vAddr.size()));
    return;
}

[brunoerg]

Force-pushed addressing #30688 (comment)

[Eunovo]

Code Review ACK

[Eunovo]

Tested ACK
I tested this using the method below but I'm a little uncomfortable with the magic number in the test. It might be worth it to expose the MAX_ADDR_TO_SEND and use in tests.

Ran FUZZ=addrman build_fuzz/src/test/fuzz/fuzz qa-assets/fuzz_seed_corpus/addrman for master and FUZZ=addrman src/test/fuzz/fuzz qa-assets/fuzz_seed_corpus/addrman for the current PR for 5mins each and got the following output (last few lines shown):

On master

#6076   REDUCE cov: 3887 ft: 21925 corp: 1246/57Mb lim: 1013425 exec/s: 27 rss: 1044Mb L: 801/1013406 MS: 1 EraseBytes-
#6547   NEW    cov: 3887 ft: 21926 corp: 1247/58Mb lim: 1013425 exec/s: 29 rss: 1044Mb L: 716612/1013406 MS: 1 CMP- DE: "\000\377\377SSSSSSSSSSSSS"-
#6959   REDUCE cov: 3887 ft: 21926 corp: 1247/58Mb lim: 1013425 exec/s: 29 rss: 1044Mb L: 696/1013406 MS: 2 CopyPart-EraseBytes-
#7484   NEW    cov: 3887 ft: 21933 corp: 1248/58Mb lim: 1013425 exec/s: 29 rss: 1044Mb L: 262394/1013406 MS: 5 ChangeBit-PersAutoDict-PersAutoDict-ChangeASCIIInt-ChangeByte- DE: "y\000\000\000"-"y\000\000\000"-
#7885   REDUCE cov: 3887 ft: 21933 corp: 1248/58Mb lim: 1013425 exec/s: 30 rss: 1044Mb L: 42585/1013406 MS: 1 EraseBytes-
#7922   REDUCE cov: 3887 ft: 21933 corp: 1248/58Mb lim: 1013425 exec/s: 30 rss: 1044Mb L: 436055/1013406 MS: 2 ChangeBit-EraseBytes-
#7988   REDUCE cov: 3887 ft: 21933 corp: 1248/58Mb lim: 1013425 exec/s: 30 rss: 1044Mb L: 64/1013406 MS: 1 EraseBytes-
#8192   pulse  cov: 3887 ft: 21933 corp: 1248/58Mb lim: 1013425 exec/s: 30 rss: 1044Mb
#8350   REDUCE cov: 3887 ft: 21933 corp: 1248/58Mb lim: 1013425 exec/s: 29 rss: 1044Mb L: 119516/1013406 MS: 2 InsertByte-EraseBytes-
#8358   REDUCE cov: 3887 ft: 21933 corp: 1248/58Mb lim: 1013425 exec/s: 29 rss: 1044Mb L: 2385/1013406 MS: 3 ChangeASCIIInt-PersAutoDict-EraseBytes- DE: "\377\377\377\377"-
#8446   NEW    cov: 3887 ft: 21934 corp: 1249/58Mb lim: 1013425 exec/s: 29 rss: 1044Mb L: 122257/1013406 MS: 3 ShuffleBytes-InsertRepeatedBytes-CopyPart-
#8767   REDUCE cov: 3887 ft: 21934 corp: 1249/58Mb lim: 1013425 exec/s: 30 rss: 1044Mb L: 119/1013406 MS: 1 EraseBytes-
#8768   REDUCE cov: 3887 ft: 21934 corp: 1249/58Mb lim: 1013425 exec/s: 30 rss: 1044Mb L: 222/1013406 MS: 1 EraseBytes-
#8993   REDUCE cov: 3887 ft: 21934 corp: 1249/58Mb lim: 1013425 exec/s: 30 rss: 1044Mb L: 107/1013406 MS: 5 InsertRepeatedBytes-CopyPart-EraseBytes-ChangeByte-EraseBytes-

On this PR

#20159  REDUCE cov: 3884 ft: 21207 corp: 1202/37Mb lim: 1048551 exec/s: 70 rss: 750Mb L: 66/1042462 MS: 1 EraseBytes-
#20465  REDUCE cov: 3884 ft: 21207 corp: 1202/37Mb lim: 1048551 exec/s: 71 rss: 750Mb L: 192/1042462 MS: 1 EraseBytes-
#20537  REDUCE cov: 3884 ft: 21207 corp: 1202/37Mb lim: 1048551 exec/s: 71 rss: 750Mb L: 30270/1042462 MS: 2 ChangeByte-EraseBytes-
#20721  NEW    cov: 3884 ft: 21216 corp: 1203/38Mb lim: 1048551 exec/s: 71 rss: 750Mb L: 320476/1042462 MS: 4 InsertByte-ChangeByte-ChangeASCIIInt-CMP- DE: "\000\000\177\324\242\177>\247"-
#21069  REDUCE cov: 3884 ft: 21216 corp: 1203/38Mb lim: 1048551 exec/s: 71 rss: 750Mb L: 119/1042462 MS: 3 ChangeBit-ShuffleBytes-EraseBytes-
#21131  REDUCE cov: 3884 ft: 21216 corp: 1203/38Mb lim: 1048551 exec/s: 71 rss: 750Mb L: 84506/1042462 MS: 2 ShuffleBytes-EraseBytes-
#21309  NEW    cov: 3884 ft: 21217 corp: 1204/38Mb lim: 1048551 exec/s: 71 rss: 750Mb L: 500634/1042462 MS: 3 PersAutoDict-CrossOver-CopyPart- DE: "-connect"-
#21330  NEW    cov: 3884 ft: 21218 corp: 1205/38Mb lim: 1048551 exec/s: 72 rss: 750Mb L: 214/1042462 MS: 1 CMP- DE: "\000\000RP\000rq="-
#21377  REDUCE cov: 3884 ft: 21218 corp: 1205/38Mb lim: 1048551 exec/s: 72 rss: 750Mb L: 120595/1042462 MS: 2 ChangeBit-EraseBytes-
#21630  REDUCE cov: 3884 ft: 21218 corp: 1205/38Mb lim: 1048551 exec/s: 72 rss: 750Mb L: 51001/1042462 MS: 3 ChangeBit-InsertRepeatedBytes-EraseBytes-
#21687  REDUCE cov: 3884 ft: 21218 corp: 1205/38Mb lim: 1048551 exec/s: 72 rss: 750Mb L: 26311/1042462 MS: 2 ChangeByte-EraseBytes-

Which shows a more than 2x increase in the exec/s on my machine.

NB I use timeout 300s to run both commands for exactly 300s

[brunoerg]

Force-pushed increasing the number of iterations from 100 to 1000 (the original value is 10'000). With this value, I could see faster execs (115 exec/s) and same coverage.

[brunoerg]

Just provided the fuzz inputs for testing: brunoerg/qa-assets#1

[Eunovo]

I retested this PR @de85ebe against master using the qa-assets https://github.com/bitcoin-core/qa-assets from and brunoerg/qa-assets#1

This PR vs Master on https://github.com/bitcoin-core/qa-assets

#2419155        REDUCE cov: 3972 ft: 22225 corp: 1718/24Mb lim: 964552 exec/s: 90 rss: 1015Mb L: 94/949713 MS: 4 CopyPart-ChangeASCIIInt-ChangeBinInt-EraseBytes-

Master

#2762232        REDUCE cov: 4008 ft: 22596 corp: 1684/30Mb lim: 883365 exec/s: 65 rss: 1032Mb L: 568/868887 MS: 1 EraseBytes-

On brunoerg/qa-assets#1

#4862507        REDUCE cov: 3969 ft: 22094 corp: 1675/25Mb lim: 979714 exec/s: 87 rss: 987Mb L: 1895/936269 MS: 1 EraseBytes-

Master

#1164418        REDUCE cov: 4005 ft: 22237 corp: 1418/33Mb lim: 880321 exec/s: 65 rss: 981Mb L: 4738/805664 MS: 2 ChangeByte-EraseBytes-

I ran the fuzz tests for several hours, while this PR is always faster, there is still a clear drop in coverage, even on the qa-assets provided in brunoerg/qa-assets#1

[marcofleon]

@Eunovo I'm not seeing that reduction in coverage when I run the target. What sanitizers are you using? Also, what does the coverage say if you do -runs=1? (instead of letting it run for a while)

[brunoerg]

I just moved it to draft since I'm doing more experiments yet.

[maflcko]

Are you still working on this?

[brunoerg]

Closing for now. Focusing on other things.