PVS-Studio terkesan dengan kualitas kode Abbyy NeoML

image1.png


Baru-baru ini, ABBYY menerbitkan kode sumber untuk kerangka kerja NeoML-nya. Kami ditawari untuk memeriksa perpustakaan ini menggunakan PVS-Studio. Ini adalah proyek yang menarik dari sudut pandang analisis, jadi kami tidak menundanya di back burner. Anda tidak perlu banyak waktu untuk membaca artikel ini, karena proyeknya berkualitas tinggi :).



Kode sumber NeoML dapat ditemukan di GitHub . Kerangka kerja ini adalah lintas platform dan dirancang untuk menerapkan model pembelajaran mesin. Ini digunakan oleh pengembang ABBYY untuk memecahkan masalah penglihatan komputer, pemrosesan bahasa alami, termasuk pemrosesan gambar, analisis dokumen, dan sebagainya. Saat ini, bahasa pemrograman seperti C ++, Java, Objective-C didukung, dan Python harus segera ditambahkan ke daftar ini. Bahasa utama di mana kerangka itu sendiri ditulis adalah C ++.



Jalankan analisis



Sangat mudah untuk menjalankan analisis pada kerangka ini. Setelah menghasilkan proyek Visual Studio di CMake, saya meluncurkan analisis PVS-Studio di Visual Studio untuk proyek-proyek yang menarik bagi kami dari Solution, tidak termasuk perpustakaan pihak ketiga. Selain NeoML sendiri, solusinya juga termasuk perpustakaan dari ABBYY seperti NeoOnnx dan NeoMathEngine. Saya juga memasukkannya ke dalam daftar proyek yang analisisnya diluncurkan.



Hasil analisa



Tentu saja, saya benar-benar ingin menemukan beberapa kesalahan yang mengerikan, tetapi ... kodenya ternyata sangat bersih dan peringatan hanya diterima, tidak ada. Sangat mungkin bahwa analisis statis telah digunakan dalam pengembangan. Banyak peringatan adalah pemicu dari diagnostik yang sama untuk bagian kode yang serupa.



Sebagai contoh, dalam proyek ini sangat umum untuk memanggil metode virtual dari konstruktor. Secara umum, ini adalah pendekatan yang berbahaya. Diagnostik V1053 merespons kasus-kasus seperti ini : Memanggil fungsi virtual 'foo' di konstruktor / destruktor dapat menyebabkan hasil yang tidak terduga saat runtime . Sebanyak 10 peringatan seperti itu dikeluarkan. Baca lebih lanjut tentang mengapa ini adalah praktik berbahaya dan masalah apa yang ditimbulkannya dalam artikel ini oleh Scott Myers "Jangan Pernah Memanggil Fungsi Virtual selama Konstruksi atau Penghancuran . "Namun, tampaknya pengembang mengerti apa yang mereka lakukan, dan tidak ada kesalahan.



Ada juga 11 peringatan V803 diagnostik tingkat menengah dari bagian" optimasi mikro ". Diagnosis ini merekomendasikan untuk mengganti kenaikan postfix dengan yang awalan. jika nilai sebelumnya dari iterator tidak digunakan. Dalam kasus kenaikan postfix, objek sementara tambahan dibuat. Tentu saja, ini bukan kesalahan, hanya detail kecil . Jika diagnostik tersebut tidak menarik, maka ketika menggunakan analisa, Anda dapat mematikannya. Nah, pada prinsipnya, seperangkat "mikro- optimasi "dan dimatikan secara default.



Sebenarnya, saya pikir Anda mengerti bahwa karena kita sampai pada analisis hal-hal sepele seperti kenaikan iterator dalam artikel, maka semuanya secara umum baik-baik saja dan kita tidak tahu harus mencari kesalahan apa.



Sangat sering, beberapa diagnostik mungkin tidak dapat diterapkan atau tidak menarik bagi pengguna, dan lebih baik tidak memakan kaktus, tetapi menghabiskan sedikit waktu menyiapkan alat analisis. Anda dapat membaca lebih lanjut tentang langkah-langkah yang harus diambil untuk segera mendekati pemicu penganalisa yang paling menarik di artikel kami " Bagaimana cara cepat melihat peringatan menarik yang dikeluarkan oleh penganalisa PVS-Studio untuk kode C dan C ++? "



Di antara pemicu dari bagian " optimisasi mikro "ada juga peringatan diagnostik V802 yang menarik, yang merekomendasikan mengatur bidang struktur dalam urutan ukuran jenis, yang memungkinkan untuk mengurangi ukuran struktur.



V802 Pada platform 64-bit, ukuran struktur dapat dikurangi dari 24 hingga 16 byte dengan mengatur ulang bidang sesuai dengan ukurannya dalam urutan menurun. HierarchicalClustering.h 31



struct CParam {
  TDistanceFunc DistanceType; 
  double MaxClustersDistance;
  int MinClustersCount; 
};


Hanya menukar bidang MaxClustersDistance tipe ganda dan enumerator DistanceType dapat mengurangi ukuran struktur dari 24 hingga 16 byte.



struct CParam {
  double MaxClustersDistance;
  int MinClustersCount; 
  TDistanceFunc DistanceType; 
};


TDistanceFunc adalah enum , jadi ukurannya setara dengan int atau kurang, sehingga harus dipindahkan ke ujung struktur.



Sekali lagi, ini bukan kesalahan, tetapi jika pengguna tertarik untuk melakukan optimasi mikro atau jika mereka, pada prinsipnya, penting untuk proyek ini, pemicu analisis semacam itu memungkinkan Anda untuk dengan cepat menemukan tempat untuk setidaknya refactoring primer.



Secara umum, semua kode ditulis dengan rapi dan dapat dibaca , tetapi diagnostik V807 menunjuk ke beberapa tempat yang dapat dibuat sedikit lebih optimal dan dapat dibaca. Biarkan saya memberi Anda contoh paling ilustratif:



V807 Penurunan kinerja. Coba buat referensi untuk menghindari penggunaan ungkapan yang sama berulang kali. GradientBoostFullTreeBuilder.cpp 469



image3.png


Panggilan ke curLevelStatistics [i] -> ThreadStatistics [j] dapat diganti dengan panggilan ke variabel terpisah. Tidak ada panggilan ke metode kompleks dalam rantai ini, jadi mungkin tidak ada banyak optimasi di sini, tetapi kode, menurut pendapat saya, akan dibaca lebih mudah dan lebih kompak. Selain itu, dengan dukungan kode ini, akan terlihat jelas di masa mendatang bahwa perlu untuk mengatasi secara tepat indeks ini dan tidak ada kesalahan. Untuk kejelasan, saya akan memberikan kode dengan pengganti variabel:



auto threadStatistics = curLevelStatistics[i]->ThreadStatistics[j];

if(threadStatistics.FeatureIndex != NotFound ) {
  if(   threadStatistics.Criterion > criterion
     || ( .... ))
  {
    criterion = threadStatistics.Criterion;
    curLevelStatistics[i]->FeatureIndex    = threadStatistics.FeatureIndex;
    curLevelStatistics[i]->Threshold       = threadStatistics.Threshold;
    curLevelStatistics[i]->LeftStatistics  = threadStatistics.LeftStatistics;
    curLevelStatistics[i]->RightStatistics = threadStatistics.RightStatistics;
  }
}


Kesimpulan



Seperti yang Anda lihat, dalam hal analisis statis, basis kode kerangka kerja ini ternyata sangat bersih.



Harus dipahami bahwa satu rangkaian analisis pada proyek yang dikembangkan aktif secara lemah mencerminkan perlunya analisis statis, karena banyak kesalahan, terutama jika mereka kritis, telah terdeteksi dengan cara lain, tetapi jauh lebih memakan waktu dan sumber daya intensif. Poin ini dianalisis secara lebih rinci dalam artikel " Kesalahan yang tidak dapat ditemukan oleh analisis kode statis, karena tidak digunakan ".



Tetapi bahkan dengan fakta ini dalam pikiran, beberapa peringatan dikeluarkan pada NeoML, dan saya ingin menyatakan rasa hormat saya untuk kualitas kode dalam proyek ini, terlepas dari apakah pengembang menggunakan analisis statis atau tidak.





Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan terjemahan: Victoria Khanieva. PVS-Studio Terkesan oleh Kualitas Kode ABBYY NeoML .



All Articles