erigontech / erigon

Ethereum implementation on the efficiency frontier https://erigon.gitbook.io
GNU Lesser General Public License v3.0
3.15k stars 1.13k forks source link

Caplin: to set boundaries - no how much goroutines can be spawned by for loop #12510

Open AskAlexSharov opened 1 month ago

AskAlexSharov commented 1 month ago

Found many patterns like:

wg := WaitGroup{}
for {
  wg.Add(1)
  go func() {
     defer wg.Done(1)
  }
}
wg.Wait()

Examples: getDutiesProposer connectWithAllPeers batchVerifyAttestations VerifyAgainstIdentifiersAndInsertIntoTheBlobStore

This pattern spawns unlimited amount of goroutines. All "unlimited" things are bad - because:


let's replace it by next pattern:

g := &errgroup.Group{}
g.SetLimit(runtime.GOMAXPROCS(-1))
for {
  g.Go(func() error {
  })
}
if err := g.Wait(); err != nil {
    return err 
})

Can set any limit you like.

Giulio2002 commented 1 week ago

meh - need to think about this

stevemilk commented 1 week ago

I write a simple benchmark test, here's the benchmark result:

CPU Cores: 10
Tasks number: 1000

=== CPU-Bound Task Benchmark ===
1 goroutines: 335.22075ms
10 goroutines: 39.401667ms
100 goroutines: 38.951541ms
1000 goroutines: 39.360083ms
2000 goroutines: 40.008459ms

=== I/O-Bound Task Benchmark ===
1 goroutines: 10.976698875s
10 goroutines: 1.10276125s
100 goroutines: 109.47375ms
1000 goroutines: 12.514292ms
2000 goroutines: 12.588917ms

so we can infer that:

for above examples, there's one i/o-bound task connectWithAllPeers, others are cpu-bound. maybe this could help.

shotasilagadze commented 5 days ago

I would add one more thing here, we could have more goroutines than CPUs for just fairness. Also, goroutines don't mean CPU threads, we still have restricted number of OS threads, goroutines are just mapped on those and scheduling/mapping overhead for goroutines is not a big deal.

So, associating goroutines with OS threads and their scheduling wouldn't be a good start for refactoring most of the code.

I will look into those places and check if we are unnecessarily starting goroutines though.