[インデックス 19591] ファイルの概要
このコミットは、src/pkg/strings/replace.go
と src/pkg/strings/replace_test.go
の2つのファイルを変更しています。主な変更は、strings
パッケージ内の Replacer
型が使用していた sync.Pool
の利用を元に戻すものです。
コミット
commit a200e0b8fda46dadb1474248f21d44a16bd92dd2
Author: Rui Ueyama <ruiu@google.com>
Date: Sun Jun 22 15:26:30 2014 -0700
undo CL 101330053 / c19c9a063fe8
sync.Pool is not supposed to be used everywhere, but is
a last resort.
««« original CL description
strings: use sync.Pool to cache buffer
benchmark old ns/op new ns/op delta
BenchmarkByteReplacerWriteString 3596 3094 -13.96%
benchmark old allocs new allocs delta
BenchmarkByteReplacerWriteString 1 0 -100.00%
LGTM=dvyukov
R=bradfitz, dave, dvyukov
CC=golang-codereviews
https://golang.org/cl/101330053
»»»
LGTM=dave
R=r, dave
CC=golang-codereviews
https://golang.org/cl/102610043
GitHub上でのコミットページへのリンク
https://github.com/golang/go/commit/a200e0b8fda46dadb1474248f21d44a16bd92dd2
元コミット内容
このコミットは、以前のコミット CL 101330053 / c19c9a063fe8
を元に戻すものです。元々のコミットの目的は、strings
パッケージの Replacer
が内部で使用するバッファを sync.Pool
を使ってキャッシュすることで、パフォーマンスを向上させることでした。
元のコミットの説明は以下の通りです。
strings: use sync.Pool to cache buffer
benchmark old ns/op new ns/op delta
BenchmarkByteReplacerWriteString 3596 3094 -13.96%
benchmark old allocs new allocs delta
BenchmarkByteReplacerWriteString 1 0 -100.00%
LGTM=dvyukov
R=bradfitz, dave, dvyukov
CC=golang-codereviews
https://golang.org/cl/101330053
このベンチマーク結果は、sync.Pool
の導入により BenchmarkByteReplacerWriteString
の実行時間が約14%短縮され、メモリ割り当てが1回から0回に削減されたことを示しています。
変更の背景
このコミットの背景には、sync.Pool
の適切な使用方法に関するGo言語コミュニティの設計思想があります。コミットメッセージに明記されているように、「sync.Pool
はあらゆる場所で使用されるべきではなく、最後の手段としてのみ使用されるべきである」という原則に基づいています。
以前のコミットでは、strings.Replacer
の WriteString
メソッド内で一時的なバイトスライスバッファを sync.Pool
から取得・返却することで、メモリ割り当てを削減し、パフォーマンスを向上させようとしました。しかし、sync.Pool
は一時的なオブジェクトの再利用を目的としていますが、そのプールされたオブジェクトはガベージコレクションの対象となり得るため、永続的なキャッシュとしては適していません。また、プールの乱用はコードの複雑性を増し、デバッグを困難にする可能性があります。
strings.Replacer
のバッファは、特定の状況下では頻繁に再利用される可能性がありますが、その性質上、sync.Pool
の「最後の手段」という位置づけには合致しないと判断されたと考えられます。つまり、パフォーマンス上の利点があったとしても、sync.Pool
の設計意図と長期的な保守性の観点から、その利用は適切ではないと判断され、元に戻す決定がなされました。
前提知識の解説
sync.Pool
sync.Pool
はGo言語の標準ライブラリ sync
パッケージで提供される型で、一時的なオブジェクトを再利用するためのプールです。主な目的は、頻繁に生成・破棄される一時オブジェクトのメモリ割り当てを減らし、ガベージコレクションの負荷を軽減することで、アプリケーションのパフォーマンスを向上させることです。
- 仕組み:
sync.Pool
は、Get()
メソッドでプールからオブジェクトを取得し、Put()
メソッドでオブジェクトをプールに戻します。プールが空の場合、New
フィールドに設定された関数が呼び出され、新しいオブジェクトが生成されます。 - 特徴と注意点:
- 一時的なオブジェクトの再利用:
sync.Pool
は、一時的に使用され、その後すぐに不要になるオブジェクト(例: バッファ、一時的なデータ構造)の再利用に適しています。 - ガベージコレクションの対象: プール内のオブジェクトは、ガベージコレクタによっていつでも回収される可能性があります。したがって、
sync.Pool
はキャッシュとして使用すべきではありません。プールから取得したオブジェクトが必ずしも利用可能である保証はなく、Get()
がnil
を返す可能性もあります(その場合はNew
関数が呼ばれる)。 - スレッドセーフ: 複数のGoroutineから安全にアクセスできるように設計されています。
- 「最後の手段」: コミットメッセージにもあるように、
sync.Pool
はパフォーマンス上のボトルネックが明確であり、他の最適化手法が尽きた場合の「最後の手段」として検討されるべきです。安易な導入は、コードの可読性や保守性を損なう可能性があります。
- 一時的なオブジェクトの再利用:
strings.Replacer
strings.Replacer
は、Go言語の strings
パッケージで提供される型で、文字列内の複数の部分文字列を効率的に置換するための機能を提供します。一度 NewReplacer
関数で置換ルール(古い文字列と新しい文字列のペア)を設定すると、そのルールに基づいて複数の文字列に対して置換操作を高速に実行できます。内部的には、置換対象の文字列を効率的に検索するためのデータ構造(例: トライ木)を使用しています。
io.Writer
io.Writer
はGo言語の io
パッケージで定義されているインターフェースです。
type Writer interface {
Write(p []byte) (n int, err error)
}
このインターフェースは、バイトスライス p
を書き込む Write
メソッドを1つだけ持ちます。Write
メソッドは、書き込まれたバイト数 n
と、発生したエラー err
を返します。ファイル、ネットワーク接続、標準出力など、バイトデータを書き込むことができるあらゆるもの(「シンク」と呼ばれることもあります)が io.Writer
インターフェースを実装できます。strings.Replacer
の WriteString
メソッドは、この io.Writer
を引数に取ることで、置換結果を直接ファイルやネットワークなどに書き出すことを可能にします。
Goのメモリ管理とガベージコレクション (GC)
Go言語は自動メモリ管理(ガベージコレクション)を採用しています。開発者は手動でメモリを解放する必要がありません。GoのGCは、到達可能性に基づいて不要になったメモリを自動的に回収します。
- アロケーションのコスト: メモリを割り当てる(
make
やnew
など)こと自体にはコストがかかります。これは、メモリ領域の確保、ゼロ初期化、GCによる追跡などに関連します。 - GCの負荷: GCは、不要なオブジェクトを特定し、メモリを再利用するプロセスです。GCが実行されると、一時的にプログラムの実行が停止(ストップ・ザ・ワールド)したり、CPUリソースを消費したりすることがあります。
sync.Pool
とGC:sync.Pool
は、一時的なオブジェクトの再利用を促進することで、アロケーションの頻度を減らし、結果としてGCの負荷を軽減する効果が期待されます。しかし、前述の通り、プールされたオブジェクトもGCの対象となり得るため、GCの動作を完全に制御するものではありません。
技術的詳細
このコミットは、strings.Replacer
の WriteString
メソッドにおけるバッファ管理戦略を sync.Pool
を使用する方式から、より直接的な make
によるバッファ割り当てに戻すものです。
元のコミット (CL 101330053
) では、strings.Replacer
の WriteString
メソッドが内部で使用する一時的なバイトスライスバッファ (buf
) を sync.Pool
から取得し、処理後にプールに戻していました。これにより、BenchmarkByteReplacerWriteString
ベンチマークでは、ns/op
が約14%改善し、メモリ割り当てが1回から0回に削減されるという顕著なパフォーマンス向上が見られました。これは、頻繁に呼び出される WriteString
メソッドにおいて、毎回新しいバッファを割り当てるオーバーヘッドと、それに伴うGCの負荷を sync.Pool
が軽減したためです。
しかし、このコミット (a200e0b8fda46dadb1474248f21d44a16bd92dd2
) では、その変更が元に戻されました。その理由は、sync.Pool
が「あらゆる場所で使用されるべきではなく、最後の手段である」というGo言語の設計原則にあります。strings.Replacer
のバッファは、一時的ではあるものの、その再利用のパターンが sync.Pool
の意図する「一時的なオブジェクトの再利用によるGC負荷軽減」というよりも、より一般的なバッファ管理の範疇に属すると判断された可能性があります。sync.Pool
の乱用は、コードの複雑性を増し、プールされたオブジェクトがGCによって回収される可能性を考慮する必要があるため、デバッグや理解を困難にする可能性があります。
具体的な変更点としては、bufferPool
という sync.Pool
のインスタンスが削除され、WriteString
メソッド内でバッファが必要になるたびに make([]byte, bufsize)
を使って新しいバイトスライスが割り当てられるようになりました。bufsize
は、デフォルトで 32 << 10
(32KB) と設定されていますが、入力文字列 s
の長さがそれよりも小さい場合は len(s)
に調整されます。これにより、不要に大きなバッファを割り当てることを避けています。
また、テストファイル src/pkg/strings/replace_test.go
から TestWriteStringError
というテストケースが削除されています。このテストは、io.Writer
がエラーを返した場合の WriteString
の挙動を検証するものでしたが、sync.Pool
の利用と関連していたか、あるいは変更後のコードパスでは不要になったため削除されたと考えられます。さらに、algorithmTestCases
というテストケースの定義がグローバル変数から TestPickAlgorithm
関数内のローカル変数に移動されています。これは、テストのスコープを適切に限定し、グローバルな状態への依存を減らすための一般的なリファクタリングです。
この変更は、パフォーマンス上のわずかな後退を許容してでも、Go言語の標準ライブラリにおける sync.Pool
の利用ガイドラインを厳守し、コードベース全体の健全性と保守性を優先するという設計思想を反映しています。
コアとなるコードの変更箇所
src/pkg/strings/replace.go
--- a/src/pkg/strings/replace.go
+++ b/src/pkg/strings/replace.go
@@ -4,10 +4,7 @@
package strings
-import (
- "io"
- "sync"
-)
+import "io"
// A Replacer replaces a list of strings with replacements.
type Replacer struct {
@@ -454,31 +451,27 @@ func (r *byteReplacer) Replace(s string) string {
return string(buf)
}
-var bufferPool = sync.Pool{
- New: func() interface{} {
- b := make([]byte, 4096)
- return &b
- },
-}
-
func (r *byteReplacer) WriteString(w io.Writer, s string) (n int, err error) {
- bp := bufferPool.Get().(*[]byte)
- buf := *bp
+ // TODO(bradfitz): use io.WriteString with slices of s, avoiding allocation.
+ bufsize := 32 << 10
+ if len(s) < bufsize {
+ bufsize = len(s)
+ }
+ buf := make([]byte, bufsize)
+
for len(s) > 0 {
- ncopy := copy(buf, s)
+ ncopy := copy(buf, s[:])
+ s = s[ncopy:]
for i, b := range buf[:ncopy] {
buf[i] = r.new[b]
}
- s = s[ncopy:]
- var wn int
- wn, err = w.Write(buf[:ncopy])
+ wn, err := w.Write(buf[:ncopy])
n += wn
if err != nil {
- break
+ return n, err
}
}
- bufferPool.Put(bp)
- return
+ return n, nil
}
// byteStringReplacer is the implementation that's used when all the
src/pkg/strings/replace_test.go
--- a/src/pkg/strings/replace_test.go
+++ b/src/pkg/strings/replace_test.go
@@ -308,21 +308,20 @@ func TestReplacer(t *testing.T) {
}\n}\n \n-var algorithmTestCases = []struct {\n-\tr *Replacer\n-\twant string\n-}{\n-\t{capitalLetters, \"*strings.byteReplacer\"},\n-\t{htmlEscaper, \"*strings.byteStringReplacer\"},\n-\t{NewReplacer(\"12\", \"123\"), \"*strings.singleStringReplacer\"},\n-\t{NewReplacer(\"1\", \"12\"), \"*strings.byteStringReplacer\"},\n-\t{NewReplacer(\"\", \"X\"), \"*strings.genericReplacer\"},\n-\t{NewReplacer(\"a\", \"1\", \"b\", \"12\", \"cde\", \"123\"), \"*strings.genericReplacer\"},\n-}\n-\n // TestPickAlgorithm tests that NewReplacer picks the correct algorithm.\n func TestPickAlgorithm(t *testing.T) {\n-\tfor i, tc := range algorithmTestCases {\n+\ttestCases := []struct {\n+\t\tr *Replacer\n+\t\twant string\n+\t}{\n+\t\t{capitalLetters, \"*strings.byteReplacer\"},\n+\t\t{htmlEscaper, \"*strings.byteStringReplacer\"},\n+\t\t{NewReplacer(\"12\", \"123\"), \"*strings.singleStringReplacer\"},\n+\t\t{NewReplacer(\"1\", \"12\"), \"*strings.byteStringReplacer\"},\n+\t\t{NewReplacer(\"\", \"X\"), \"*strings.genericReplacer\"},\n+\t\t{NewReplacer(\"a\", \"1\", \"b\", \"12\", \"cde\", \"123\"), \"*strings.genericReplacer\"},\n+\t}\n+\tfor i, tc := range testCases {\n \t\tgot := fmt.Sprintf(\"%T\", tc.r.Replacer())\n \t\tif got != tc.want {\n \t\t\tt.Errorf(\"%d. algorithm = %s, want %s\", i, got, tc.want)\n@@ -330,23 +329,6 @@ func TestPickAlgorithm(t *testing.T) {\n \t}\n }\n \n-type errWriter struct{}\n-\n-func (errWriter) Write(p []byte) (n int, err error) {\n-\treturn 0, fmt.Errorf(\"unwritable\")\n-}\n-\n-// TestWriteStringError tests that WriteString returns an error\n-// received from the underlying io.Writer.\n-func TestWriteStringError(t *testing.T) {\n-\tfor i, tc := range algorithmTestCases {\n-\t\tn, err := tc.r.WriteString(errWriter{}, \"abc\")\n-\t\tif n != 0 || err == nil || err.Error() != \"unwritable\" {\n-\t\t\tt.Errorf(\"%d. WriteStringError = %d, %v, want 0, unwritable\", i, n, err)\n-\t\t}\n-\t}\n-}\n-\n // TestGenericTrieBuilding verifies the structure of the generated trie. There\n // is one node per line, and the key ending with the current line is in the\n // trie if it ends with a \"+\".\n```
## コアとなるコードの解説
### `src/pkg/strings/replace.go` の変更点
1. **`sync` パッケージのインポート削除**:
`import ("io", "sync")` から `import "io"` に変更され、`sync` パッケージへの依存がなくなりました。これは `sync.Pool` を使用しなくなったためです。
2. **`bufferPool` 変数の削除**:
`var bufferPool = sync.Pool{...}` という `sync.Pool` のインスタンス定義が完全に削除されました。
3. **`WriteString` メソッドの変更**:
* **バッファ取得ロジックの変更**:
* 変更前: `bp := bufferPool.Get().(*[]byte)` と `buf := *bp` を使って `sync.Pool` からバッファを取得していました。
* 変更後: `bufsize := 32 << 10` (32KB) をデフォルトのバッファサイズとし、もし入力文字列 `s` の長さが `bufsize` より小さい場合は `bufsize = len(s)` となります。そして、`buf := make([]byte, bufsize)` を使って毎回新しいバイトスライスバッファを割り当てるようになりました。これにより、`sync.Pool` を介したバッファの再利用は行われなくなりました。
* **バッファ返却ロジックの削除**:
* 変更前: `bufferPool.Put(bp)` を使って処理後にバッファをプールに戻していました。
* 変更後: この行は削除されました。新しいバッファは関数スコープ内で作成され、関数終了後にガベージコレクションの対象となります。
* **エラーハンドリングの簡素化**:
* 変更前: `var wn int; wn, err = w.Write(buf[:ncopy]); n += wn; if err != nil { break }`
* 変更後: `wn, err := w.Write(buf[:ncopy]); n += wn; if err != nil { return n, err }`
* `break` ではなく `return n, err` を使うことで、エラー発生時に即座に処理を終了し、書き込まれたバイト数とエラーを返すようになりました。これはより一般的なGoのエラーハンドリングパターンです。
* **戻り値の変更**:
* 変更前: `return` (名前付き戻り値 `n` と `err` が自動的に返される)
* 変更後: `return n, nil` (ループが正常に完了した場合、エラーがないことを明示的に `nil` で返します)
### `src/pkg/strings/replace_test.go` の変更点
1. **`algorithmTestCases` のスコープ変更**:
* 変更前: `var algorithmTestCases = []struct{...}` としてグローバル変数で定義されていました。
* 変更後: `TestPickAlgorithm` 関数内に `testCases := []struct{...}` としてローカル変数で定義されるようになりました。これにより、テストケースの定義がそのテスト関数に限定され、他のテストやグローバルな状態への影響がなくなります。
2. **`errWriter` 型と `TestWriteStringError` 関数の削除**:
* `errWriter` というカスタムの `io.Writer` 実装と、それを使用する `TestWriteStringError` テスト関数が完全に削除されました。このテストは、`WriteString` メソッドが `io.Writer` からのエラーを適切に処理するかどうかを検証するものでしたが、`sync.Pool` の利用がなくなったことや、`WriteString` のエラーハンドリングロジックが変更されたことにより、この特定のテストケースが不要になったか、あるいは別の方法でカバーされるようになったためと考えられます。
これらの変更は、`sync.Pool` の利用を元に戻すという主要な目的を達成しつつ、関連するコードのクリーンアップとテストの調整を行っています。
## 関連リンク
* GitHubコミットページ: [https://github.com/golang/go/commit/a200e0b8fda46dadb1474248f21d44a16bd92dd2](https://github.com/golang/go/commit/a200e0b8fda46dadb1474248f21d44a16bd92dd2)
* 元コミット (CL 101330053): [https://golang.org/cl/101330053](https://golang.org/cl/101330053)
* このコミットのCL (CL 102610043): [https://golang.org/cl/102610043](https://golang.org/cl/102610043)
## 参考にした情報源リンク
* Go言語 `sync.Pool` のドキュメント: [https://pkg.go.dev/sync#Pool](https://pkg.go.dev/sync#Pool)
* Go言語 `strings` パッケージのドキュメント: [https://pkg.go.dev/strings](https://pkg.go.dev/strings)
* Go言語 `io` パッケージのドキュメント: [https://pkg.go.dev/io](https://pkg.go.dev/io)
* Go言語のメモリ管理とGCに関する一般的な情報源 (例: 公式ブログ、Go Conferenceの発表など)
* Go Blog: The Go garbage collector: Prioritizing low latency and simplicity: [https://go.dev/blog/go15gc](https://go.dev/blog/go15gc)
* Go Blog: Go's work-stealing goroutine scheduler: [https://go.dev/blog/go115-gcsweep](https://go.dev/blog/go115-gcsweep)
* (注: 上記は一般的な情報源であり、このコミットの直接的な参照元ではありませんが、背景知識として関連します。)