代碼量銳減 80%,一次祖傳代碼重構實踐

此前,團隊接管並重構了十多年前的搜索鏈路中的 Query 理解祖傳代碼,代碼量減少 80%,性能、穩定性、可觀測性都得到大幅度提升,且支持自研雲和業務機房雙環境部署。本文將分享重構過程中碰到的代碼壞味道,並分析這樣寫的動機、預防和拯救措施。

作者 | 龍鑫 責編 | 夏萌

出品 | 騰訊雲開發者

背景

1.1 接手

在一次組織架構調整後,我們組接手了鏈路搜索的幾個底層基礎模塊—— QU(Query 理解)相關的三個模塊,其中包括本次重構對象 Query Optimizer ,負責根據用戶在搜索系統中輸入的 Query(查詢語句)產出切詞、詞權、緊密度、意圖識別結果。QO 在搜索鏈路中的上下游關係如下圖所示:

1.2 爲什麼重構

面對一份10年的祖傳代碼,我們選擇重構的原因主要如下:

迭代效率低:新增一個簡單的算子需要 3 人天,效率低下。

穩定性較差:不定期出現 P99 毛刺。

啓動速度慢:服務啓動需要 18 分鐘。

內存浪費多:單進程需 114 G 內存。

排查工具少:缺少多項監控和 trace 跟蹤能力。

GCC 老舊:使用 GCC 4.8,無法使用現代 C++。

無法部署到自研雲:無法和騰訊域下的類似能力做合併。

基於上述原因,也源於我們熱愛挑戰、勇於折騰,我們決定在項目完全接手後啓動重構計劃。

後面的內容將分享老代碼中的壞味道,當初這樣寫的動機、對應的預防和拯救措施及優化之後的效果。

重複的代碼

2.1 示例

下面代碼爲 gbk 與 utf8 格式的互相轉換函數。 兩個函數之間除了變量輸入順序不一樣,其他都是一樣的。

2.2 動機

我懶得提取公共代碼。 CV 大法最簡單,最快速。

2.3 預防和拯救措施

CodeCC 能掃描出部分重複代碼。

代 碼編寫過程中,優先複用已有的代碼。

提升單測覆蓋率要求。越多的重複代碼意味着,我需要寫越多的單測,逼迫自己去使用已有工具。

當兩個層次相同的類存在相同的方法時,就把方法提出來,上移到一個上層的類或者獨立的方法。比如上面的編碼函數在不同的類中都存在,最後我們將該方法提取出來了,並複用了可以共用的部分。

2.4 優化之後

過長函數

3.1 示例

你見過 1380 行代碼的函數嗎?我見過雖然其中 300 行是被註釋掉了,100行是註釋。

3.2 動機

我不想刪代碼,所以註釋代碼。爲什麼不想刪代碼,因爲還想着後面還要用。事實證明,後面沒有再用。

多寫點註釋,後面的人就能看懂。

當一個函數代碼行數越來越多的時候,我不願意去承擔重構的風險。如果要新加一個功能,在主流程加上我的邏輯是最保險的。如果我要去改動別人的代碼,即使只是提取出來作爲一個函數,我需要承擔更多的風險。

3.3 預防和拯救措施

如果代碼未來還會有用,建議加上開關,而不是註釋。又或者可以先刪除,未來需要使用時,通過 git 記錄找回來。

如果需要寫很多註釋來表明某個邏輯,可以提出該段邏輯爲一個獨立的函數。

項目框架搭建過程中,想清楚每個接口的職責,不要讓某個接口大包大攬,最後成爲垃圾場。

使用 CodeCC 規則進行檢查。

3.4 優化之後

臃腫的類

4.1 示例

作爲一個負責請求處理的類,不僅包括 HTTP 服務實例、緩存實例,還需要執行幾十個具體的策略邏輯,實在是有點不堪重負。當我閱讀完這個類的代碼,我覺得我讀完了一本書,要睡着了。

4.2 動機

最開始,僅僅有一個策略邏輯。這個邏輯放在請求處理類中時,我覺得理所應當。然而,後面的策略同學覺得和我的邏輯應該差不多,直接 CV。

我們有時候賦予一個類太大的職責範圍了,以至於我們後面無論想到啥,似乎都符合我們給這個類的人設,理所當然的把邏輯加進去。

4.3 預防和拯救措施

每個類應該在註釋中說明該類職責。當類中實例過多時,應當想辦法拆解,把一部分職責委託爲其他類。

仔細思考是否可以提取出一個新的類,比如,將數個彼此相關的變量提取到一個新的類,放到一起。

4.4 優化之後

過長參數列表

5.1 示例

一個方法56個參數,我真怕自己傳錯參數了。

5.2 動機

其實我本可以將參數放在 config 裡面,但是爲了保險,別人怎麼做,我就怎麼做。當有一個壞的開始,後面 就會有人不斷重複這個錯誤的示範。反正沒有代碼 CR,只要能 RUN 就行。

5.3 預防和拯救措施

編碼過程中關注參數列表的長度。

關注單測增量覆蓋率,讓 CV 付出代價。畢竟在單測中,你需要填充這個過長的參數列表,如果你自己都受不了,那別人也會受不了。

傳遞對象,讓方法從對象中獲取它需要的參數。

5.4 優化之後

令人迷惑的臨時字段

6.1 示例

下面的代碼表示:如果 HaveSecond 爲 true 的時候,i 及 i+1個單詞的weight*100/2。給你十分鐘,你能明白這個含義嗎?爲什麼看不懂?因爲 is_second 這個變量的含義很繞。

6.2 動機

在循環中,如果需要在特定條件下,對 i 及 i+1 個元素進行操作,我擔心會溢出。所以,我把該操作分爲多步,並用一個臨時變量表明接下來需要進行操作。

6.3 預防和拯救措施

注意代碼可讀性,每個變量需要有它特定的含義。同時,注意最少代碼原則,思考清楚,這個變量真的是需要的嗎?

如果兩個邏輯需要通過一個變量來進行連接,那爲什麼不直接把這兩個邏輯合在一起,消除這個變量。

6.4 優化之後

傳入參數範圍過大

7.1 示例

worker 是 整個服務輸出結果的存儲對象,其中 proc_node 存儲了所有算子的中間輸出結果。然而 worker 指針和 proc_node 指針被傳遞到了多個函數。這些函數真的需要這麼多的信息?

7.2 動機

我懶得去思考不同接口的數據依賴。於是將所有數據塞到同一個結構體,要用什麼直接拿,要寫入什麼信息,直接寫入。反正都是串行執行,不存在多線程問題。

7.3 預防和拯救措施

遵循最少知識原則,只給該接口它所需要的數據。

7.4 優化之後

不必要的串行

8.1 示例

如下代碼要進行兩次切詞,一次帶標點符號的切詞,一次爲去除標點符號的切詞。兩者其實可以並行。

8.2 動機

反正已經有請求級別的並行了,任務處理級別的並行意義不大。

萬一有多線程安全問題,等於給自己挖坑。

8.3 預防和拯救措施

多關注性能。CR 過程中對於主流程添加的邏輯,審視可能帶來的耗時增加。上線灰度時,留心監控中被調耗時和內存利用率。

如果請求處理過程中,存在多個可以並行的任務,建議使用 DAG 進行任務註冊和任務運行。

8.4 優化之後

基於 DAG 進行調度,多個子任務之間並行處理。最終主流程從 13.19 ms 優化到 9.71 ms,優化 26%。

被忽視的編譯 warning

9.1 示例

下面代碼沒有 return,升級 gcc 後(gcc4.8.5->8.3.1),調用函數訪問到了異常值,coredump 了。

下面代碼 sprintf 寫入 char 數組的時候,沒有給\0保留位置,最終棧空間因爲越界被寫壞,函數局部變量值都變成異常值,導致後面的數組訪問到隨機內存空間。

9.2 動機

warning,不是 error,能編譯通過,我覺得問題不大,有點 warning 很正常。

目前代碼能正確運行,我也不想動。

編譯過程中輸出太多信息了,我不可能去看每一條輸出信息。

9.3 預防和拯救措施

編譯時打開 -Wall -Werror 編譯選項,將 warning 變成 error,中斷編譯,讓 warning 得到足夠重視。

9.4 優化之後

魔法數字和常量

10.1 示例

誰來告訴我 ‘43000’ ‘4300’是什麼?

10.2 動機

針對 badcase 進行特殊處理,我懶得寫註釋了,也不想花時間定義常量。

10.3 預防和拯救措施

針對常量,遵循代碼規範,使用常量定義,並添加註釋。

10.4 優化之後

下面常量定義已脫敏,不代表真實含義。

過長的 if

11.1 示例

下面這個判斷 執行了100多次。

11.2 動機

經驗比較少,不瞭解用查表進行優化。

11.3 預防和拯救措施

針對很長的判斷,儘量使用查表替代判斷。

11.4 優化之後

總結

重構中碰到的部分代碼壞味道分享到此爲止。體驗了這次大掃除的辛苦,我發誓,我以後再也不偷懶了,多動腦、少 CV、少走捷徑,手動狗頭。

本文經授權轉載自「騰訊雲開發者」公衆號,點擊「閱讀原文」可以直達本文原文!