-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Concurrency explainer to docs #1
base: main
Are you sure you want to change the base?
Conversation
docs/concurrency.md
Outdated
is map[int]string | ||
} | ||
|
||
func (m *C...Map) Set(s string, i int) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix this shorthand (I didn't want to type it all out in Slack).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a couple of the links to fill in, then we can officially start the docs folder with this 👍
docs/concurrency.md
Outdated
m.is[i] = s | ||
} | ||
|
||
// func GetI, func GetS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Write these functions out for clarity func (m *CMap) GetI(i int) string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
### 5. Batch channel values together where it makes sense | ||
|
||
It’s a small optimization, but there is overhead in every `select` and in every channel that the runtime maintains, and there is overhead in switching active goroutines, so if `Parent` sends multiple values to `Child` and those values are somehow related, prefer `toChildCh <- oneValWithMultipleOptionalFields` over `fooToChildCh <- foo; barToChildCh <- bar; bazToChildCh <- baz` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link to https://github.com/rollchains/gordian/blob/fd375361b8a69146183c5f9b670d799c6e6e5066/tm/tmengine/tmelink/networkviewupdate.go#L10 as a practical example of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Gordian we use unbuffered channels in two primary cases: | ||
|
||
- boring “signals” with closed channels (will be discussed further in next major point) | ||
- batching collections of updates, where we need to be certain that the receiver has the data we sent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Real example: mirror kernel to gossip strategy https://github.com/rollchains/gordian/blob/fd375361b8a69146183c5f9b670d799c6e6e5066/tm/tmengine/internal/tmmirror/internal/tmi/kernel.go#L1107 and
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
- Panics are typically unrecoverable because they happen in a goroutine other than the caller; if you have a panic they are a bit more intensive to find the root cause when it isn’t obvious | ||
- Returning values from the control loop goroutine means that you usually have to have a request-response pair of types; the response returns a _**copy**_ of the writable data that the main goroutine owns. If used improperly, this can lead to a lot of garbage creation, but there are multiple mitigation strategies to avoid that. | ||
- Clean execution of this pattern involves widespread use of `select` and generally, every send and every receive is going to be in a `select` at least also watching a root `context.Context` for cancellation. So naturally there are going to be some helper functions for the common case of sending to or receiving from one channel while watching that root context. (TODO: add links to or examples of these helper functions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These examples are
https://github.com/rollchains/gordian/blob/fd375361b8a69146183c5f9b670d799c6e6e5066/internal/gchan/send.go#L15
https://github.com/rollchains/gordian/blob/fd375361b8a69146183c5f9b670d799c6e6e5066/internal/gchan/send.go#L30
https://github.com/rollchains/gordian/blob/fd375361b8a69146183c5f9b670d799c6e6e5066/internal/gchan/send.go#L46
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @mark-rushakoff