Codewalk之Golang并发代码回顾
Golang Concurrency Code Review of Codewalk
我正在尝试了解 Golang 并发的最佳实践。我读了 O'Reilly 关于 Go 的并发性的书,然后回到了 Golang Codewalks,特别是这个例子:
https://golang.org/doc/codewalk/sharemem/
这是我希望与您一起复习的代码,以便更多地了解 Go。我的第一印象是这段代码打破了一些最佳实践。这当然是我(非常)没有经验的意见,我想讨论这个过程并获得一些见解。这不是关于谁对谁错的问题,请保持友善,我只是想分享我的观点并获得一些反馈。也许这个讨论会帮助其他人明白为什么我错了并教他们一些东西。
我完全明白这段代码的目的是教初学者,而不是完美的代码。
问题 1 - 没有 Goroutine 清理逻辑
func main() {
// Create our input and output channels.
pending, complete := make(chan *Resource), make(chan *Resource)
// Launch the StateMonitor.
status := StateMonitor(statusInterval)
// Launch some Poller goroutines.
for i := 0; i < numPollers; i++ {
go Poller(pending, complete, status)
}
// Send some Resources to the pending queue.
go func() {
for _, url := range urls {
pending <- &Resource{url: url}
}
}()
for r := range complete {
go r.Sleep(pending)
}
}
main 方法无法清理 Goroutine,这意味着如果这是库的一部分,它们就会被泄露。
问题 2 - 作者没有生成频道
我将其作为最佳实践阅读,创建、写入 和清理[=59= 的逻辑] 通道应由单个实体(或实体组)控制。这背后的原因是写者在写到一个封闭的频道时会恐慌。因此,最好由编写者创建通道、写入通道并控制何时关闭。如果有多个写入器,他们可以与一个 WaitGroup 同步。
func StateMonitor(updateInterval time.Duration) chan<- State {
updates := make(chan State)
urlStatus := make(map[string]string)
ticker := time.NewTicker(updateInterval)
go func() {
for {
select {
case <-ticker.C:
logState(urlStatus)
case s := <-updates:
urlStatus[s.url] = s.status
}
}
}()
return updates
}
这个函数不应该负责创建更新通道,因为它是通道的 reader,而不是作者。这个频道的作者应该创建它并将它传递给这个函数。基本上是对功能说“我将通过此频道向您传递更新”。但是,这个函数正在创建一个通道,不清楚谁负责清理它。
问题 3 - 异步写入通道
这个函数:
func (r *Resource) Sleep(done chan<- *Resource) {
time.Sleep(pollInterval + errTimeout*time.Duration(r.errCount))
done <- r
}
此处引用:
for r := range complete {
go r.Sleep(pending)
}
这似乎是个糟糕的主意。当这个通道关闭时,我们将有一个 goroutine 在我们够不到的地方睡觉,等待写入那个通道。假设这个 goroutine 睡了 1 小时,当它醒来时,它会尝试写入一个在清理过程中关闭的通道。这是为什么频道的作者应该负责清理过程的另一个例子。在这里,我们有一位完全空闲的作家,他不知道频道何时关闭。
请
如果我遗漏了该代码中的任何问题(与并发相关),请列出它们。它不一定是一个 objective 问题,如果您出于任何原因以不同的方式设计代码,我也有兴趣了解它。
这段代码的最大教训
对我来说,我从审查这段代码中得到的最大教训是通道的清理和写入通道必须同步。它们必须在同一个 for{} 中或至少以某种方式进行通信(可能通过其他通道或基元)以避免写入已关闭的通道。
是main
的方法,不用清理。当main
returns时,程序退出。如果这不是 main
,那么你是对的。
没有适合所有用例的最佳实践。您在此处显示的代码是一种非常常见的模式。该函数创建一个 goroutine 和 returns 一个通道,以便其他人可以与该 goroutine 通信。没有规定必须如何创建通道的规则。但是没有办法终止那个 goroutine。这种模式非常适合的一个用例是从
数据库。该通道允许流式传输数据,因为它是从
数据库。在这种情况下,通常还有其他终止方式
goroutine,就像传递上下文一样。
同样,对于频道应该如何设置没有硬性规定created/closed。通道可以保持打开状态,不再使用时将被垃圾收集。如果用例需要,通道可以无限期打开,你担心的场景永远不会发生。
- 当您询问这段代码是否是库的一部分时,是的,在库函数中生成没有清理的 goroutine 是不好的做法。如果这些 goroutine 执行库的记录行为,那么调用者 不知道该行为何时会发生是有问题的 。如果您有任何典型的“即发即弃”行为,则应该是呼叫者选择何时忘记它。例如:
func doAfter5Minutes(f func()) {
go func() {
time.Sleep(5 * time.Minute)
f()
log.Println("done!")
}()
}
有道理,对吧?当您调用该函数时,它会在 5 分钟后执行某些操作。问题是很容易像这样误用这个函数:
// do the important task every 5 minutes
for {
doAfter5Minutes(importantTaskFunction)
}
乍一看,这似乎不错。我们每 5 分钟执行一次重要任务,对吗?实际上,我们正在快速生成许多 goroutine,可能在它们开始下降之前就消耗掉了所有可用内存。
我们可以实现某种回调或通道以在任务完成时发出信号,但实际上,函数应该像这样简化:
func doAfter5Minutes(f func()) {
time.Sleep(5 * time.Minute)
f()
log.Println("done!")
}
现在调用者可以选择如何使用它:
// call synchronously
doAfter5Minutes(importantTaskFunction)
// fire and forget
go doAfter5Minutes(importantTaskFunction)
- 按理说这个功能也应该改变。正如您所说,作者应该有效地拥有频道,因为他们应该是关闭频道的人。这个通道读取函数坚持创建它读取的通道这一事实实际上强迫自己进入上面提到的这种糟糕的“即发即弃”模式。请注意函数需要如何从通道读取,但在读取之前它还需要 return 通道。因此,它必须将阅读行为放在一个新的、非托管的 goroutine 中,以允许自己立即 return 频道。
func StateMonitor(updates chan State, updateInterval time.Duration) {
urlStatus := make(map[string]string)
ticker := time.NewTicker(updateInterval)
defer ticker.Stop() // not stopping the ticker is also a resource leak
for {
select {
case <-ticker.C:
logState(urlStatus)
case s := <-updates:
urlStatus[s.url] = s.status
}
}
}
请注意,该函数现在更简单、更灵活且同步。之前版本真正完成的唯一一件事是,它(大部分)保证 StateMonitor
的每个实例都有自己的通道,并且不会出现多个监视器竞争读取的情况同一个频道。虽然这 可能 可以帮助您避免某些 class 错误,但它也会使函数的灵活性大大降低,并且更容易出现资源泄漏。
- 我不确定我是否真的理解这个例子,但是关闭频道的黄金法则是作者应该始终负责关闭频道。请记住这条规则,并注意有关此代码的几点:
Sleep
方法写入r
Sleep
方法是并发执行的,没有方法跟踪有多少个实例运行,它们处于什么状态等
仅根据这些要点,我们可以说程序中可能没有 任何地方 可以安全关闭 r
,因为似乎不知道会不会再用
我正在尝试了解 Golang 并发的最佳实践。我读了 O'Reilly 关于 Go 的并发性的书,然后回到了 Golang Codewalks,特别是这个例子:
https://golang.org/doc/codewalk/sharemem/
这是我希望与您一起复习的代码,以便更多地了解 Go。我的第一印象是这段代码打破了一些最佳实践。这当然是我(非常)没有经验的意见,我想讨论这个过程并获得一些见解。这不是关于谁对谁错的问题,请保持友善,我只是想分享我的观点并获得一些反馈。也许这个讨论会帮助其他人明白为什么我错了并教他们一些东西。
我完全明白这段代码的目的是教初学者,而不是完美的代码。
问题 1 - 没有 Goroutine 清理逻辑
func main() {
// Create our input and output channels.
pending, complete := make(chan *Resource), make(chan *Resource)
// Launch the StateMonitor.
status := StateMonitor(statusInterval)
// Launch some Poller goroutines.
for i := 0; i < numPollers; i++ {
go Poller(pending, complete, status)
}
// Send some Resources to the pending queue.
go func() {
for _, url := range urls {
pending <- &Resource{url: url}
}
}()
for r := range complete {
go r.Sleep(pending)
}
}
main 方法无法清理 Goroutine,这意味着如果这是库的一部分,它们就会被泄露。
问题 2 - 作者没有生成频道
我将其作为最佳实践阅读,创建、写入 和清理[=59= 的逻辑] 通道应由单个实体(或实体组)控制。这背后的原因是写者在写到一个封闭的频道时会恐慌。因此,最好由编写者创建通道、写入通道并控制何时关闭。如果有多个写入器,他们可以与一个 WaitGroup 同步。
func StateMonitor(updateInterval time.Duration) chan<- State {
updates := make(chan State)
urlStatus := make(map[string]string)
ticker := time.NewTicker(updateInterval)
go func() {
for {
select {
case <-ticker.C:
logState(urlStatus)
case s := <-updates:
urlStatus[s.url] = s.status
}
}
}()
return updates
}
这个函数不应该负责创建更新通道,因为它是通道的 reader,而不是作者。这个频道的作者应该创建它并将它传递给这个函数。基本上是对功能说“我将通过此频道向您传递更新”。但是,这个函数正在创建一个通道,不清楚谁负责清理它。
问题 3 - 异步写入通道
这个函数:
func (r *Resource) Sleep(done chan<- *Resource) {
time.Sleep(pollInterval + errTimeout*time.Duration(r.errCount))
done <- r
}
此处引用:
for r := range complete {
go r.Sleep(pending)
}
这似乎是个糟糕的主意。当这个通道关闭时,我们将有一个 goroutine 在我们够不到的地方睡觉,等待写入那个通道。假设这个 goroutine 睡了 1 小时,当它醒来时,它会尝试写入一个在清理过程中关闭的通道。这是为什么频道的作者应该负责清理过程的另一个例子。在这里,我们有一位完全空闲的作家,他不知道频道何时关闭。
请
如果我遗漏了该代码中的任何问题(与并发相关),请列出它们。它不一定是一个 objective 问题,如果您出于任何原因以不同的方式设计代码,我也有兴趣了解它。
这段代码的最大教训
对我来说,我从审查这段代码中得到的最大教训是通道的清理和写入通道必须同步。它们必须在同一个 for{} 中或至少以某种方式进行通信(可能通过其他通道或基元)以避免写入已关闭的通道。
是
main
的方法,不用清理。当main
returns时,程序退出。如果这不是main
,那么你是对的。没有适合所有用例的最佳实践。您在此处显示的代码是一种非常常见的模式。该函数创建一个 goroutine 和 returns 一个通道,以便其他人可以与该 goroutine 通信。没有规定必须如何创建通道的规则。但是没有办法终止那个 goroutine。这种模式非常适合的一个用例是从 数据库。该通道允许流式传输数据,因为它是从 数据库。在这种情况下,通常还有其他终止方式 goroutine,就像传递上下文一样。
同样,对于频道应该如何设置没有硬性规定created/closed。通道可以保持打开状态,不再使用时将被垃圾收集。如果用例需要,通道可以无限期打开,你担心的场景永远不会发生。
- 当您询问这段代码是否是库的一部分时,是的,在库函数中生成没有清理的 goroutine 是不好的做法。如果这些 goroutine 执行库的记录行为,那么调用者 不知道该行为何时会发生是有问题的 。如果您有任何典型的“即发即弃”行为,则应该是呼叫者选择何时忘记它。例如:
func doAfter5Minutes(f func()) {
go func() {
time.Sleep(5 * time.Minute)
f()
log.Println("done!")
}()
}
有道理,对吧?当您调用该函数时,它会在 5 分钟后执行某些操作。问题是很容易像这样误用这个函数:
// do the important task every 5 minutes
for {
doAfter5Minutes(importantTaskFunction)
}
乍一看,这似乎不错。我们每 5 分钟执行一次重要任务,对吗?实际上,我们正在快速生成许多 goroutine,可能在它们开始下降之前就消耗掉了所有可用内存。
我们可以实现某种回调或通道以在任务完成时发出信号,但实际上,函数应该像这样简化:
func doAfter5Minutes(f func()) {
time.Sleep(5 * time.Minute)
f()
log.Println("done!")
}
现在调用者可以选择如何使用它:
// call synchronously
doAfter5Minutes(importantTaskFunction)
// fire and forget
go doAfter5Minutes(importantTaskFunction)
- 按理说这个功能也应该改变。正如您所说,作者应该有效地拥有频道,因为他们应该是关闭频道的人。这个通道读取函数坚持创建它读取的通道这一事实实际上强迫自己进入上面提到的这种糟糕的“即发即弃”模式。请注意函数需要如何从通道读取,但在读取之前它还需要 return 通道。因此,它必须将阅读行为放在一个新的、非托管的 goroutine 中,以允许自己立即 return 频道。
func StateMonitor(updates chan State, updateInterval time.Duration) {
urlStatus := make(map[string]string)
ticker := time.NewTicker(updateInterval)
defer ticker.Stop() // not stopping the ticker is also a resource leak
for {
select {
case <-ticker.C:
logState(urlStatus)
case s := <-updates:
urlStatus[s.url] = s.status
}
}
}
请注意,该函数现在更简单、更灵活且同步。之前版本真正完成的唯一一件事是,它(大部分)保证 StateMonitor
的每个实例都有自己的通道,并且不会出现多个监视器竞争读取的情况同一个频道。虽然这 可能 可以帮助您避免某些 class 错误,但它也会使函数的灵活性大大降低,并且更容易出现资源泄漏。
- 我不确定我是否真的理解这个例子,但是关闭频道的黄金法则是作者应该始终负责关闭频道。请记住这条规则,并注意有关此代码的几点:
Sleep
方法写入r
Sleep
方法是并发执行的,没有方法跟踪有多少个实例运行,它们处于什么状态等
仅根据这些要点,我们可以说程序中可能没有 任何地方 可以安全关闭 r
,因为似乎不知道会不会再用