[インデックス 13004] ファイルの概要

このコミットは、Go言語のコードレビューダッシュボード（misc/dashboard/codereview）における、変更リスト（CL: Change List）の表示順序を維持するための修正です。具体的には、特定の条件（現在のユーザーが作成者またはレビュー担当者であるCL）に合致するCLをリストから削除する際に、既存の要素の順序を破壊しないように、スライス操作の方法を変更しています。

コミット

misc/dashboard/codereview: preserve CL ordering.

R=r
CC=golang-dev
https://golang.org/cl/6136056

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/cc9a5c3be7fb8c5935dbfa49b7926219674fa705

元コミット内容

commit cc9a5c3be7fb8c5935dbfa49b7926219674fa705
Author: David Symonds <dsymonds@golang.org>
Date:   Tue May 1 16:15:32 2012 +1000

    misc/dashboard/codereview: preserve CL ordering.
    
    R=r
    CC=golang-dev
    https://golang.org/cl/6136056
---
 misc/dashboard/codereview/dashboard/front.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/misc/dashboard/codereview/dashboard/front.go b/misc/dashboard/codereview/dashboard/front.go
index 9eb36f3143..475a663516 100644
--- a/misc/dashboard/codereview/dashboard/front.go
+++ b/misc/dashboard/codereview/dashboard/front.go
@@ -78,7 +78,8 @@ func handleFront(w http.ResponseWriter, r *http.Request) {
 			for i := len(tbl.CLs) - 1; i >= 0; i-- {
 				cl := tbl.CLs[i]
 				if cl.Author == currentPerson || cl.Reviewer == currentPerson {
-					tbl.CLs[i] = tbl.CLs[len(tbl.CLs)-1]
+					// Preserve order.
+					copy(tbl.CLs[i:], tbl.CLs[i+1:])
 					tbl.CLs = tbl.CLs[:len(tbl.CLs)-1]
 				}
 			}

変更の背景

この変更の背景には、Go言語のコードレビューシステムにおけるユーザーエクスペリエンスの改善があります。Goプロジェクトでは、Gerritベースのコードレビューシステムが使用されており、その進捗を一覧表示するダッシュボードが存在します。このダッシュボードでは、複数の変更リスト（CL）が表示されますが、特定のCLをリストから除外する際に、その順序が意図せず変更されてしまう問題がありました。

元の実装では、リストから要素を削除する際に、削除対象の要素をリストの最後の要素で上書きし、その後リストの長さを1つ減らすという一般的なGoのスライス操作が行われていました。この方法は効率的ですが、要素の相対的な順序を維持しません。例えば、[A, B, C, D]からBを削除する場合、BをDで上書きし、[A, D, C]（その後[A, D, C]の最後の要素を削除して[A, D]）となるため、CとDの相対的な順序が入れ替わってしまう可能性があります。

コードレビューダッシュボードのようなUIでは、CLの表示順序（例えば、更新日時順や重要度順など）がユーザーにとって意味を持つことが多いため、この順序が破壊されることは望ましくありませんでした。このコミットは、この順序破壊の問題を解決し、ユーザーが期待する表示順序を維持することを目的としています。

前提知識の解説

Go言語のスライス (Slice)

Go言語のスライスは、配列をラップした動的なデータ構造です。スライスは、基になる配列の一部を参照し、長さ（len）と容量（cap）を持ちます。スライスは、要素の追加や削除によって動的にサイズを変更できますが、実際には基になる配列のサイズが変更されるわけではなく、スライスが参照する範囲が変更されるか、新しい基になる配列が割り当てられることで実現されます。

スライスからの要素削除

Go言語において、スライスから要素を削除する一般的な方法はいくつかあります。

1. 順序を気にしない削除: 削除したい要素をスライスの最後の要素で上書きし、スライスの長さを1つ減らす方法です。これは最も効率的な方法ですが、要素の順序は維持されません。

   // 例: index i の要素を削除
   s[i] = s[len(s)-1] // 最後の要素で上書き
   s = s[:len(s)-1]   // 長さを減らす
   

2. 順序を維持する削除: copy関数を使用して、削除したい要素の後の要素を前にずらす方法です。これにより、要素の相対的な順序が維持されます。

   // 例: index i の要素を削除
   copy(s[i:], s[i+1:]) // i+1以降の要素をiの位置にコピー
   s = s[:len(s)-1]     // 長さを減らす
   

このコミットは、まさにこの「順序を維持する削除」の方法に切り替えることで問題を解決しています。

Go Code Review Dashboard

Goプロジェクトでは、Gerritというコードレビューシステムが広く利用されています。このシステムは、変更リスト（Change List, CL）と呼ばれる単位でコードの変更を管理し、レビュープロセスを効率化します。Goのダッシュボードは、これらのCLのステータスや担当者などを一覧で確認できるWebインターフェースであり、開発者が自身の担当するCLやレビュー中のCLを追跡するために使用されます。misc/dashboard/codereviewは、このダッシュボードのコードベースの一部を指します。

技術的詳細

このコミットの技術的な核心は、Go言語のスライス操作におけるcopy関数の利用と、それによる要素削除時の順序維持です。

元のコードでは、以下の行で要素を削除していました。

tbl.CLs[i] = tbl.CLs[len(tbl.CLs)-1]
tbl.CLs = tbl.CLs[:len(tbl.CLs)-1]

これは、ループ内でi番目の要素を、スライスの最後の要素で上書きし、その後スライスを1つ短くするという操作です。この方法は、要素の順序が重要でない場合に、メモリの再割り当てを避けるためによく使われる効率的なイディオムです。しかし、tbl.CLs[len(tbl.CLs)-1]（つまり最後の要素）がi番目の位置に移動するため、元のi番目の要素以降の要素の相対的な順序が破壊されます。

新しいコードでは、以下の行に変更されました。

copy(tbl.CLs[i:], tbl.CLs[i+1:])
tbl.CLs = tbl.CLs[:len(tbl.CLs)-1]

ここで重要なのはcopy(tbl.CLs[i:], tbl.CLs[i+1:])です。

• tbl.CLs[i:] は、スライスのi番目の要素から最後までを含むサブスライスです。これはcopy関数のコピー先（destination）となります。
• tbl.CLs[i+1:] は、スライスのi+1番目の要素から最後までを含むサブスライスです。これはcopy関数のコピー元（source）となります。

このcopy関数は、tbl.CLs[i+1:]の内容をtbl.CLs[i:]にコピーします。これにより、i+1番目以降のすべての要素が1つ前のインデックスにシフトされます。例えば、[A, B, C, D]というスライスでB（インデックス1）を削除する場合、copy(s[1:], s[2:])は[C, D]をs[1:]にコピーします。結果としてスライスは[A, C, D, D]のようになり、その後s = s[:len(s)-1]によって最後の重複要素が削除され、[A, C, D]となります。この操作により、CとDの相対的な順序が維持されます。

この変更は、ダッシュボードの表示において、CLの順序がユーザーにとって意味を持つ場合に、その順序が予期せず変更されることを防ぐために不可欠でした。特に、日付順やステータス順でソートされたリストから要素が削除される際に、そのソート順が維持されることが期待されます。

コアとなるコードの変更箇所

変更は misc/dashboard/codereview/dashboard/front.go ファイルの handleFront 関数内で行われています。

--- a/misc/dashboard/codereview/dashboard/front.go
+++ b/misc/dashboard/codereview/dashboard/front.go
@@ -78,7 +78,8 @@ func handleFront(w http.ResponseWriter, r *http.Request) {
 			for i := len(tbl.CLs) - 1; i >= 0; i-- {
 				cl := tbl.CLs[i]
 				if cl.Author == currentPerson || cl.Reviewer == currentPerson {
-					tbl.CLs[i] = tbl.CLs[len(tbl.CLs)-1]
+					// Preserve order.
+					copy(tbl.CLs[i:], tbl.CLs[i+1:])
 					tbl.CLs = tbl.CLs[:len(tbl.CLs)-1]
 				}
 			}

具体的には、79行目から80行目にかけての変更です。

コアとなるコードの解説

変更されたコードブロックは、tbl.CLsというスライスを逆順にループ処理しています。このループの目的は、現在のユーザーが作成者であるか、またはレビュー担当者であるCLをスライスから削除することです。

• for i := len(tbl.CLs) - 1; i >= 0; i--: スライスの末尾から先頭に向かってループします。逆順にループするのは、要素を削除する際にインデックスがずれる問題を避けるためです。前方から削除すると、削除後の要素のインデックスが変わり、次のイテレーションで誤った要素を参照する可能性があります。
• cl := tbl.CLs[i]: 現在のインデックスiにあるCLを取得します。
• if cl.Author == currentPerson || cl.Reviewer == currentPerson: 取得したCLの作成者またはレビュー担当者が現在のユーザーと一致するかどうかをチェックします。
• 変更前: tbl.CLs[i] = tbl.CLs[len(tbl.CLs)-1]
  • この行は、条件に合致したi番目の要素を、スライスの最後の要素で上書きしていました。これにより、i番目の要素は削除されますが、最後の要素がその位置に移動するため、スライスの順序が変更されていました。
• 変更後: copy(tbl.CLs[i:], tbl.CLs[i+1:])
  • この行は、i+1番目以降のすべての要素を、i番目の位置から始まるスライスにコピーします。これにより、i番目の要素が上書きされ、その後の要素が1つずつ前にシフトされます。結果として、i番目の要素が削除され、残りの要素の相対的な順序が維持されます。
• tbl.CLs = tbl.CLs[:len(tbl.CLs)-1]: copy操作の後、スライスの長さを1つ減らします。これは、スライスの末尾に重複した要素が残るため、それを切り捨てることで論理的な削除を完了させます。

この修正により、ダッシュボードに表示されるCLのリストから特定のCLが削除された場合でも、残りのCLの表示順序が維持されるようになり、ユーザーエクスペリエンスが向上しました。

関連リンク

• Go Code Review Dashboard: https://golang.org/cl/6136056 (このコミットが参照しているGerritの変更リスト)

参考にした情報源リンク

• A Tour of Go - Slices: https://go.dev/tour/moretypes/7
• Go Slices: usage and internals: https://go.dev/blog/slices-usage-and-internals
• Effective Go - Slices: https://go.dev/doc/effective_go#slices
• Gerrit Code Review: https://www.gerritcodereview.com/
• Go Project Dashboard (Example of a Go dashboard, though the specific misc/dashboard/codereview might be an internal tool or an older version): https://build.golang.org/ (一般的なGoプロジェクトのビルドダッシュボードの例)