usdc_reader benchmark #15303
usdc_reader benchmark #15303
Conversation
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
0xnogo
requested review from
winder,
rstout,
asoliman92,
makramkd,
dimkouv,
mateusz-sekara and
0xAustinWang
core/capabilities/ccip/ccip_integration_tests/usdcreader/usdcreader_test.go
Outdated
Show resolved
Hide resolved
| @@ -201,7 +207,147 @@ func Test_USDCReader_MessageHashes(t *testing.T) { | |||
| }) | |||
| } | |||
| } | |||
| func Benchmark_MessageHashes(b *testing.B) { | |||
There was a problem hiding this comment.
Should we add the gobench results above the test as a comment? (as a reference)
There was a problem hiding this comment.
Ideally we want to track the performance regressions somehow (maybe it's not in the scope for 1st iteration, but wanted to call it out anyway). Some time ago, I saw this tool https://pkg.go.dev/golang.org/x/perf/cmd/benchstat, but not sure how much it fits our use case
There was a problem hiding this comment.
I agree, benchmark tests provide very few impact if not compared. We could add a tool like benchstat in the CI and fail if we noticed a reg.
| }{ | ||
| {"Small_Dataset", 100, 1, 5}, | ||
| {"Medium_Dataset", 10_000, 1, 10}, | ||
| {"Large_Dataset", 100_000, 1, 50}, |
There was a problem hiding this comment.
Nice, was there any major difference between these 3 datasets?
There was a problem hiding this comment.
// Benchmark_MessageHashes/Small_Dataset-14 3723 272421 ns/op 126949 B/op 2508 allocs/op
// Benchmark_MessageHashes/Medium_Dataset-14 196 6164706 ns/op 1501435 B/op 20274 allocs/op
// Benchmark_MessageHashes/Large_Dataset-14 7 163930268 ns/op 37193160 B/op 463954 allocs/op
It's going linearly. I think that the remappings we have (first the messageSentEvents and then out) are a big offender (apart for the GetQuery).
|
|
||
| // Create log entry | ||
| logs = append(logs, logpoller.Log{ | ||
| EvmChainId: ubig.New(big.NewInt(int64(uint64(source)))), |
There was a problem hiding this comment.
hmm, is that right? We should use chain_id, not the chain selectors which is stored in source, right?
There was a problem hiding this comment.
The simulated backend is created with the source chain. The only part we are using the ChainID is for the transactor creation (as we don't have any other choice here, geth is complaining about it).
Requires
Supports