[Storage] Refactor badger cache and all the client code to use generics

[nozim]

Re: #3932

[Kay-Zee]

Thanks for the PR @nozim, I've gone ahead and kicked-off the CI for the branch, and will try to have someone look over the PR soon.

In the mean time, i had to update the branch with latest master to update a dependency that was causing problems for CI, and that seems to have broken tidy.

[durkmurder]

Looks good, did you try measuring performance impact of usings generics instead of interface{} ? We still haven't reached consensus on this topic inside the team.

I would be interested to see at least synthetic benchmarks so we could compare old implementation vs new. Running localnet benchmark should also give some insights. Ideally I would like to see both.

[nozim]

@durkmurder good question, will do a small benchmark :)

[nozim]

Added small example comparing to interface implementation. @durkmurder fyi :)

This is one of results:

goos: darwin
goarch: amd64
pkg: github.com/onflow/flow-go/storage/badger
cpu: VirtualApple @ 2.50GHz
BenchmarkInterface-8   	 1676054	       694.5 ns/op
BenchmarkGenerics-8    	 2030983	       588.9 ns/op
PASS
ok  	github.com/onflow/flow-go/storage/badger	11.798s

[durkmurder]

@nozim Thank you, this is good to go. Can you remove those benchmarks and run lint and tidy?

FYI, you can try passing actual type instead of any and it will be even faster, it's very recognizable if you move RandomBytes outside the loop.

I will approve this after you cleanup it.
Good job!

[durkmurder]

@nozim just to make it clear, we don't want to merge the benchmark(and iface cache implementation) in master since we don't plan to use previous cache implementation.

[nozim]

@durkmurder yes, totally makes sense. Will clean it up as well ;)

[codecov-commenter]

Codecov Report

Merging #4565 (9d68658) into master (2528190) will decrease coverage by 4.52%.
Report is 323 commits behind head on master.
The diff coverage is 56.36%.

@@            Coverage Diff             @@
##           master    #4565      +/-   ##
==========================================
- Coverage   56.25%   51.73%   -4.52%     
==========================================
  Files         653      734      +81     
  Lines       64699    66142    +1443     
==========================================
- Hits        36396    34219    -2177     
- Misses      25362    29295    +3933     
+ Partials     2941     2628     -313     

Flag	Coverage Δ
unittests	51.73% <56.36%> (-4.52%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
cmd/scaffold.go	14.83% <0.00%> (+0.06%)	⬆️
fvm/environment/env.go	100.00% <ø> (ø)
model/flow/identifierList.go	22.36% <0.00%> (+0.84%)	⬆️
model/verification/chunkDataPackRequest.go	43.75% <0.00%> (-6.25%)	⬇️
module/component/component.go	89.92% <ø> (ø)
module/metrics/access.go	0.00% <0.00%> (ø)
module/metrics/herocache.go	0.00% <0.00%> (ø)
module/metrics/network.go	0.00% <0.00%> (ø)
module/metrics/noop.go	0.00% <0.00%> (ø)
module/metrics/observer.go	0.00% <ø> (ø)
... and 61 more

... and 412 files with indirect coverage changes

[peterargue]

Thanks for cleaning this up @nozim! added a few small stylistic comments, but looks good.

[nozim]

@peterargue thanks for review, will address all of the notes )

[peterargue]

👍

[durkmurder]

@nozim can you fix last lint errors so I can approve? We can merge right after

[durkmurder]

🚢

[nozim]

seems like a glitch in engine unit test?