Conversation
| if level%2 == 0 { | ||
| (*zigzagLevelOrderValues)[level] = append((*zigzagLevelOrderValues)[level], node.Val) | ||
| } else { | ||
| (*zigzagLevelOrderValues)[level] = append([]int{node.Val}, (*zigzagLevelOrderValues)[level]...) |
|
|
||
| #### 2b reverse呼ばない | ||
| - 参考: https://github.com/seal-azarashi/leetcode/pull/26/files#diff-39117f806e1c76b27224df2000d2200427b950646feb7d63be476d764d29dfa8R14 | ||
| - 関数呼び出しのオーバーヘッドがなくなる |
There was a problem hiding this comment.
手元で実験してみたところ、4nsという数字が出てきました
実験方法は、slices.Reverseの実行開始から終了までの時間と、slices.Reverseの中身の開始から終了までの時間をそれぞれ計測し、差分を出しました。後者の場合の方が時間がかかる場合もあり、かなり分散が大きかったので、実質差はないだろうというのが結論になりました
start := time.Now()
slices.Reverse(currentLevelNodeValues)
functionCallTime += time.Since(start)start := time.Now()
for i, j := 0, len(currentLevelNodeValues)-1; i < j; i, j = i+1, j-1 {
currentLevelNodeValues[i], currentLevelNodeValues[j] = currentLevelNodeValues[j], currentLevelNodeValues[i]
}
functionCallTime += time.Since(start)There was a problem hiding this comment.
遠い記憶を掘り出してt検定をしてみたらp値0.95 > 0.05で両者に有意差がないという帰無仮説が受諾されました
There was a problem hiding this comment.
そうですね。
適当に検索して出てきたこれも 2 ns くらいといっていますね。
https://billglover.me/2018/09/17/how-expensive-is-a-go-function-call/
| - 気にしたこと: slices.Reverseの処理が重くならないかどうか確認した。 | ||
| slices.Reverseは線形時間で実行できる。 | ||
| 入力条件より、木のノード数は高々2000で、最大ノード数を持つレベルのノード数は1000未満。 | ||
| 要素数1000のスライスに対するReverseは、CPUのクロック周波数1e8psを考慮すると、1000 / 1e8 = 1e-5 = 10μs。 |
There was a problem hiding this comment.
このクロック周波数はどのような環境を想定していますか？現在のCPUは1GHzくらいは普通にあるのかなという感覚値でいます。
There was a problem hiding this comment.
ここの記述は、以前nodchipさんにされた指摘 #9 (comment) を参考にして算出していました
CPU の動作周波数は数 GHz 程度です。これは 1 秒間に数十億回機械語が実行されることを表します。 C++ をコンパイルして機械語に変換したときのオーバーヘッドを考慮すると、 1 秒間に 1～10 億ステップ程度という数字が出てきます。
正直ちゃんと環境を想定できていたわけではなかったです。自分の使っているM1チップについて調べたところ、3.2GHzでした
There was a problem hiding this comment.
環境を想定しましょうということではなく、10^8だと100MHzで結構遅いなと思ったのでどういうマシンを想定しているんだろうと気になったというのが背景になります。ただ自分の使っているPCのメモリやコア数などは知っておくと良いというか、普通は興味をもつはずみたいなことを講師陣の方はいうと思いますし、自分もそう思います。
There was a problem hiding this comment.
i486 の1994年頃のモデルが 100MHz です。
2005年にインテルから出ている初期のマルチコア CPU が Pentium D が 3 GHz 前後です。
周波数は、光の速度 約 3*10^8 m/s との関係で、3 GHz だと1クロックに 10 cm になってきてそろそろ色々と限界といわれていたので、コアを増やす方向に動いていったというのが歴史です。
There was a problem hiding this comment.
クロック周波数は最大でも9GHzくらいなんですね。
https://www.itmedia.co.jp/pcuser/articles/2310/22/news029.html
| nextLevelNodes = append(nextLevelNodes, node.Right) | ||
| } | ||
| } | ||
| if level%2 == 1 { |
There was a problem hiding this comment.
for len(currentLevelNodes) > 0 のループを終えたあとに、まとめて奇数レベルだけ slices.Reverse したほうが、ネストが浅くなり、読みやすくなるかもしれません。
There was a problem hiding this comment.
なるほどです。実装してみました
func zigzagLevelOrder(root *TreeNode) [][]int {
if root == nil {
return [][]int{}
}
zigzagLevelNodes := [][]int{}
currentLevelNodes := []*TreeNode{root}
for len(currentLevelNodes) > 0 {
currentLevelNodeValues := []int{}
nextLevelNodes := []*TreeNode{}
for _, node := range currentLevelNodes {
currentLevelNodeValues = append(currentLevelNodeValues, node.Val)
if node.Left != nil {
nextLevelNodes = append(nextLevelNodes, node.Left)
}
if node.Right != nil {
nextLevelNodes = append(nextLevelNodes, node.Right)
}
}
zigzagLevelNodes = append(zigzagLevelNodes, currentLevelNodeValues)
currentLevelNodes = nextLevelNodes
}
for i, _ := range zigzagLevelNodes {
if i%2 == 1 {
slices.Reverse(zigzagLevelNodes[i])
}
}
return zigzagLevelNodes
}
| slices.Reverse(currentLevelNodeValues) | ||
| } | ||
| order = append(order, currentLevelNodeValues) | ||
| level++ |
There was a problem hiding this comment.
好みですが、数値型の level の代わりに、boolean 型の変数を使ってもいいと思いました。
| #### 2b reverse呼ばない | ||
| - 参考: https://github.com/seal-azarashi/leetcode/pull/26/files#diff-39117f806e1c76b27224df2000d2200427b950646feb7d63be476d764d29dfa8R14 | ||
| - 関数呼び出しのオーバーヘッドがなくなる | ||
| - その代わり読みにくい |
There was a problem hiding this comment.
個人的には気になるほどの読みにくさは感じませんでした。ここをどうするかについてはオーダーに影響するので、上の oda さんのレビューに返答するのと合わせて、読みやすさのために処理効率を犠牲にしていいのか考えてもいいと思います。
There was a problem hiding this comment.
step1のコードと2bのコードの実行時間を、深さ10、要素数2^10-1個の木で計測したところ、2bの方が平均6μs短かったです。一応t検定してみたらp値0.395 > 0.05で優位差はなさそうでした
それから2bのコードを読み返してみたのですが、たしかにstep1と読みやすさの観点でも優位差はないのかなと感じました
最終的に一番すっきりかけているのが https://github.com/hroc135/leetcode/pull/26/files#r1825776653 だと思うので、実行時間的にも問題ないと思うので、これが自分は一番好きかなと思いました
| } | ||
| result = append(result, currentLevelNodeValues) | ||
| level++ | ||
| currentLevelNodes = nextLevelNodes |
There was a problem hiding this comment.
なかなか難しいですが、currentLevelNodesとcurrentLevelNodeValuesがよく似てるので一瞬わからなくなりそうですね。
ValuesCurrentLevelNodesとかだとどうでしょう。
There was a problem hiding this comment.
たしかにそうですね、思い切ってcurrentLevelを省いてnodesとvalues(nodeValues)にしてしまうのもありかもしれません
6 participants
https://leetcode.com/problems/binary-tree-zigzag-level-order-traversal/description/