程式碼審查“查”什麼(3)：效能

在程式碼審查系列的第三篇，我們將涉及就效能而言審查程式碼時需要留意什麼。

和所有的架構或設計領域一樣，一個系統效能的非功能性要求應該在前期就確定下來。無論你是工作在一個響應要求在納秒級別的低延遲交易系統上，還是在建立一個需要響應客戶的線上商店，或者正在編寫一個電話應用去管理 “待辦事項”列表，對“太慢”都要有相應的認識（譯者注：環境不同，含義不同）。

讓我們來討論一些影響效能的事情，審查者可以在程式碼審查時留意。

首先

這部分功能有硬性的效能需求嗎？

這部分審查的程式碼是否屬於一個以前釋出的服務層級協議（Service Level Agreement SLA）？或者在需求中有規定必需的效能指標嗎？

如果最初的需求來自於一個bug —— “登入介面載入太慢”，最初的開發者應該搞清楚一個合理的載入時間是多少 —— 否則審查者或者作者如何相信這個優化後的速度滿足需求？

如果有，有測試去檢查嗎？

任何對效能敏感的系統都應該有自動化測試，來保證可以滿足公佈的 SLA（在 10 ms 內為所有訂單需求提供服務）。如果沒有這些，你就只有等到客戶投訴時才知道你沒有滿足 SLA。 這不僅讓使用者感覺很糟，而且還會導致了不必要的罰款和費用。在這系列的上一篇裡，已經詳細討論了程式碼審查的測試部分。

功能修改或新功能對已有效能測試結果有不良影響嗎？

如果你會定期執行效能測試（或者你有一套可以按需要執行的測試集），應該檢查對效能有嚴格要求部分的新程式碼，確保它不會造成系統效能下降。 這可以是一個自動化的流程，但效能測試通常不像單元測試那麼普遍地被整合到 CI 環境中，所以值得強調這一審查步驟。

程式碼審查中沒有硬性的效能需求要怎麼做？

如果經過數小時的痛苦，只節省了一點點 CPU 週期，這樣的優化實在划不來。但有些事情如果審查者檢查了，就可以保證程式碼不會存在常見的效能缺陷 （完全可以避免）。檢查列表的其他部分，看是不是有簡單的方法可以防止未來可能出現的效能問題。

服務或應用外的呼叫開銷很大

在自己的應用之外使用需要跨網路躍點的系統和使用一個優化很差的 equals() 方法相比，前者開銷更大。請考慮這些：（譯者注：網路躍點 network hop 即路由。一個路由為一個躍點。傳輸過程中需要經過多個網路，每個被經過的網路裝置點（有能力路由的）叫做一個躍點，地址就是它的 IP）

資料庫呼叫

最大的麻煩可能躲在抽象後面，比如 ORM（Object Relational Mapping，物件關係對映）。但在程式碼審查時，你應該能夠找到效能問題的常見原因，就像一個迴圈裡的多次資料庫呼叫 —— 例如 ，載入一個 ID 列表，然後基於每個 ID 查詢資料庫裡的對應專案。

（↑↑↑ 點選可檢視大圖，下同）

例如 ，19 行看起來沒有什麼問題（無辜的），但它在一個 for 迴圈裡 —— 你不知道這行程式碼會導致多少次資料庫呼叫。

不必要的網路呼叫

如同資料庫，有時遠端服務也被過度使用。就像有些地方使用了多次遠端呼叫，其實一次就足夠了，或者可以用批處理或快取避免開銷巨大的網路呼叫。另一個和資料庫類似的是，一個抽象有時候會隱藏一個實際在呼叫遠端 API的方法呼叫。

移動程式、可穿戴程式過於頻繁地呼叫後端

這基本上和“不必要的網路呼叫”類似，但在移動裝置上還增加了一個問題，不必要的呼叫不僅降低效能，而且還很耗電。

高效且有效果地使用資源

除了注意如何使用網路資源外，審查者還可以檢視其他的資源使用，來確定可能的效能問題。

程式碼通過鎖來訪問共享程式碼嗎？這樣會導致效能下降和死鎖嗎？

鎖是效能的殺手，特別是在多執行緒環境下。考慮這樣的方式：僅有一個執行緒寫入或改變數值，而其他執行緒可以任意讀取；或者使用無鎖演算法。

程式碼會造成記憶體洩漏嗎？

一些 Java 中的常見原因是：可變靜態欄位，使用 ThreadLocal 和 ClassLoader。

應用程式的記憶體會無限增加嗎？

這和記憶體洩漏不同 —— 記憶體洩漏是由於垃圾收集器不能回收不再使用的物件。但任何語言，包含那些沒有垃圾收集機制的語言，都可以建立無限增長的資料結構。作為審查者，如果你看到新的值不斷被加入一個列表或者對映表（Map） ，問一下這個列表或者對映表是否或者何時被釋放或者減少。

上面的程式碼中我們可以看到，所有來自 Twitter 的訊息都被加入一個對映表。如果我們仔細檢查這個類，我們發現 allTwitterUsers 對映表從來沒有減少過，TwitterUser中的 tweets 列表也沒有。這個對映表可能很快就變得很大，這取決於我們監控使用者的數量和我們新增 tweets 的頻率。

程式碼有關閉連線或流嗎？

一不小心就會忘記關閉連線、檔案或網路流。當你審查其他人（無論什麼語言）的程式碼時，確保每個使用的檔案、網路或資料庫連線都有被正確地關閉。

上面程式碼很幸運地編譯通過，最初的作者很容易忽視這個問題。作為審查者你應該注意 Connection、Statement 和 ResultSet 這些物件在方法退出前都要關閉。在 Java 7 中通過 try-with-resources 可以很容易做到這一點。下面的截圖顯示了作者利用 try-with-resources 修改程式碼後程式碼審查的結果。

資源池配置正確嗎？

一個環境的最優配置受很多因素的影響，作為審查者你不可能馬上知道，就好像一個資料庫連線池的大小是否合適。但是有些事情你可以很容易判斷，比如這個池子太小（大小為1）或者太大（數以百萬計的執行緒）。優化這些值需要經過測試，測試環境越接近真實環境越好，但如果池子（例如執行緒池或者連線池）真的太大，在程式碼審查時可以抓出這個常見問題。邏輯表明越大越好，但每個物件都會消耗資源，物件太多通常會造成管理的開銷比帶來的好處大很多。如果有疑問，最好先使用預設設定。沒有使用預設設定的程式碼需要通過某些測試或計算，來驗證數值是合理的 。

審查者容易發現警告標誌

某些型別的程式碼可以立即顯示一個可能的效能問題。這取決於使用的語言和函式庫（請讓我們知道，在你們環境下對“程式碼異味”的評價）。

（譯者注：程式碼異味 code smells，程式碼中的任何可能導致深層次問題的症狀都可以叫做程式碼異味。程式碼異味包括重複程式碼、巨型類、方法過長等。）

反射

在Java中使用反射比不使用要慢很多。如果你檢查包含反射的程式碼，問一下反射是不是一定需要。

上面的截圖顯示了一個審查者在 Upsource 裡點選一個方法確認它來自哪裡，可以看到這個方法來自 java.lang.reflect 包，這就應該是一個警告標誌。

超時

當你審查程式碼時，你可能不知道一個操作的正常超時應該是多少，但應該這樣考慮“如果超時，系統中的其他部分會受到什麼影響？”。作為審查者你應該考慮最壞的情況 —— 如果 5 分鐘超時到了，應用程式會卡住嗎？如果超時被設定為1秒，最壞會發生什麼？如果程式碼的作者無法判斷超時的大小，而你作為審查者也不知道如何選擇一個值，這時最好讓懂行的人蔘與進來。不要等到你的使用者向你抱怨效能問題。

並行性

程式碼中有使用多個執行緒來執行一個簡單的操作嗎？除了增加時間和複雜性外，卻沒有帶來效能的提高？現代Java中，並行性比直接建立執行緒更加微妙：程式碼有使用Java 8 炫酷的並行流機制，但是卻沒有從並行性中受益嗎？例如在並行流中處理非常簡單的操作，可能會比在一個順序流上執行慢很多。

上面程式碼中增加的並行性沒有給我們帶來任何好處 —— 這個流作用在 Tweet 上，上面一個字串不會超過140個字元。就把那麼幾句話的操作並行化，也許不會改善效能，反而拆分成並行執行緒的開銷會更大。

正確性

這些事情不一定會影響系統的效能，但它們就會影響到正確性，因為它們很大程度上和執行在多執行緒環境有關。

多執行緒環境下的程式碼使用了正確的資料結構嗎？

上面的程式碼中，作者在第 12 行用一個 ArrayList 儲存所有的會話（session）。但這程式碼特別是 onOpen 方法會被多個執行緒呼叫，sessions 欄位就必須是一個執行緒安全的資料結構。這種情況下有多種選擇：可以使用Vector，也可以用 Collections.synchronizedList()建立一個執行緒安全列表（List），但最好的選擇是用CopyOnWriteArrayList，因為對這個列表的修改通常遠低於對它的讀取。

程式碼容易出現競態條件嗎？

多執行緒環境下，編寫程式碼很容易造成微妙的競爭問題。例如：

雖然遞增程式碼只有一行（第16 行），但從程式碼讀取值到設定新值的這段時間裡，另一個執行緒很可能已經增加了 orders 的值。作為審查者，要注意 get 和 set 組合都不是原子操作。

程式碼中對鎖的使用正確嗎？

對於競態條件，作為審查者你應該要檢查程式碼，不允許多個執行緒用可能會引發衝突的方式修改數值。應該要使用同步、鎖或者原子變數來控制修改的程式碼塊。

程式碼的效能測試有價值嗎？

例如，一不小心就會寫出糟糕的微基準測試。或者如果測試使用的資料和量產資料不一樣，也會得到一個錯誤的結果。

快取

雖然快取可以避免過多的外部請求，它也會面臨自己的挑戰。如果審查的程式碼使用了快取，你應該注意一些常見的問題，例如不正確的快取項失效。

程式碼級優化

如果你在審查程式碼，同時你也是一名開發者，下面的內容中可能會有你想建議的優化方法。作為團隊，你們應該很清楚效能對你們有多重要，這些優化方法對你們的程式碼是否有幫助。

大部分公司不會開發低延遲的應用程式，這些優化很可能都屬於過早的優化。

• 程式碼中是否使用了不必要的同步、鎖？如果程式碼總是執行在單執行緒下，加鎖只會帶來額外開銷。
• 程式碼中是否使用了不必要的執行緒安全資料結構？例如，可以用ArrayList 替換 Vector 嗎？
• 程式碼中是否使用了在常見操作上效能很差的資料結構？例如，使用了連結串列結構，卻經常搜尋其中的一項。
• 程式碼是否使用了鎖或者同步機制，而實際上可以用原子變數替代？
• 程式碼可以得益於延遲載入嗎？
• if 語句或其他邏輯語句能通過短路機制進行優化嗎？比如把最快的計算放在條件的開始？
• 有很多的字串格式嗎？可以更有效率嗎？
• log 語句有使用字串格式化嗎？有用檢查log級別的 if 語句包起來或者使用的日誌提供者可以進行延遲操作？

上面程式碼只會在 logger 被設定為 FINSET 時才會記錄訊息。但無論這條訊息是否真的被記錄下來，每次都會執行這個開銷很大的String.format 操作。

可以像上面程式碼那樣優化效能，即確保只在 log 級別等於某一數值時，才把訊息被寫入在 log。

Java 8中，可以使用 lambda 表示式來提高效能，而不是使用 if。由於使用到 lambda，除非訊息真的被記錄下來，String.format 不會執行。這應該是Java 8推薦的方法。

涉及到效能有很多要考慮的事情……

正如我在第一篇中列出程式碼審查需要留意的事情，本文強調了其中的一些關注點，可能是你們團隊在審查時會持續檢查的。這取決於你們專案的效能需求。

雖然本文是面向所有的開發人員，但大部分例子都是使用Java或 JVM。我最後介紹一些簡單的事情，是在審查 Java程式碼時需要留意的，這樣可以讓JVM 而不是你自己去優化程式碼：

• 編寫小的方法和類。
• 保持邏輯簡單 —— 不要使用深入巢狀的 if 或 迴圈。

程式碼越容易讓人閱讀，JIT 編譯器越有可能理解你的程式碼從而去優化它 。 如果程式碼看上去容易理解和整潔，它的效能也可能很好 —— 這應該很容易在程式碼審查時發現。

總結

談到效能，你要理解通過程式碼審查，某些方面可以快速得到改善（比如，沒有必要的資料庫呼叫），有些方面也很誘人（就像程式碼級優化）但很可能不會給你們的系統帶來很大的改善。

打賞支援我翻譯更多好文章，謝謝！

打賞譯者