nevalang / neva

🌊 Dataflow programming language with static types and implicit parallelism. Compiles to native code and Go
https://nevalang.org
MIT License
85 stars 7 forks source link

Sync: Add WaitGroup for counting lock use cases #628

Closed Catya3 closed 1 month ago

Catya3 commented 1 month ago

Add example and e2e test

On using Go's sync.WaitGroup internally

WaitGroup panics when the count gets below 0 so we have to be careful to take the count first, then take all the signals.

This may lead to panics in arithmetic use cases I.e. when passing len(a)-len(b) -> wg; wg -> :done. This is input dependent on the lengths of a and b. User needs to guard against this case.

emil14 commented 1 month ago
  1. wdyt about name sync.Wg and just putting in comment that wg stands for WaitGroup? It will allow to nodes {sync.Wg} without manual alias
  2. I propose 3 alternative APIs because of 2 reasons 1) this one looks too flexible (I'm not sure about this, it's just my experience with wg in Go) and 2) I'm afraid about it might be unsafe. Here they are:

Less Flexible And Safe

(count int, sig any) (sig any)

In Go we have wg.Done() that works exactly like this

Safe and Flexible 1

(count int, add int) (sig any)

Safe and Flexible 2

(count int, sub int) (sig any)

sub port works exactly like add but inverted, it accepts positive integers to subtract (because it looks like we gonna end up by always passing negative ints)

Catya3 commented 1 month ago

You have not described how you expect count to work in your proposed example. Should count be accepted only once? Should it update the internal count? Should it add or subtract from the internal count?

Catya3 commented 1 month ago

I can rewrite the example to use Go's sync.WaitGroup internally, but know that WaitGroup panics if the count is less than 0. This makes it very difficult to use for the implementation in Neva because -1 can and often does reach the counter before N which will cause a panic. We will need a temporary Int64 to hold negative numbers to avoid panic. It is much easier to leave the current implementation.

emil14 commented 1 month ago

You have not described how you expect count to work in your proposed example. Should count be accepted only once? Should it update the internal count? Should it add or subtract from the internal count?

True

I was thinking about this semantics (simplified code), wdyt?

for {
  c := <-count
  for c>0 {
    <-sig
    c--
  }
  ->sig
}
Catya3 commented 1 month ago

It's done the signature is now (count, sig) -> (sig)