Clever Geek Handbook

Abbyy NeoMLコード品質に感銘を受けたPVS-Studio

image1.png


先日、ABBYYはNeoMLフレームワークのソースコードを公開しました。PVS-Studioを使用してこのライブラリをチェックするように提案されました。これは分析の観点から興味深いプロジェクトであるため、脇に置かなかった。プロジェクトは高品質なので、この記事を読むのにそれほど時間はかかりません:)。



NeoML ソースコードはGitHubにあります。このフレームワークはクロスプラットフォームであり、機械学習モデルを実装するように設計されています。ABBYY開発者は、コンピュータービジョンの問題、画像処理やドキュメント分析などの自然言語処理を解決するために使用します。現在、C ++、Java、Objective-Cなどのプログラミング言語がサポートされており、Pythonはまもなくこのリストに追加される予定です。フレームワーク自体が作成された主な言語はC ++です。



分析を実行する



このフレームワークでの分析の実行は非常に簡単でした。CMakeでVisual Studioプロジェクトを生成した後、サードパーティのライブラリを除いて、ソリューションから関心のあるプロジェクトのPVS-Studio分析をVisual Studioで起動しました。NeoML自体に加えて、ソリューションには、NeoOnnxやNeoMathEngineなどのABBYYのライブラリも含まれていました。また、分析を開始したプロジェクトのリストにもそれらを含めました。



分析結果



もちろん、私は本当にひどい間違いを見つけたかったのですが...コードは十分にクリーンで、警告は何も受け取られませんでした。開発はすでに静的分析を使用している可能性があります。警告の多くは、コードの同様のセクションで同じ診断によってトリガーされました。



たとえば、このプロジェクトでは、コンストラクタから仮想メソッドが呼び出されることがよくあります。一般的に、これは危険なアプローチです。V1053このような場合に診断応答：コンストラクタ/デストラクタの「foo」で仮想関数の呼び出しは、実行時に予期しない結果につながる可能性が。このような警告が合計10件発行されました。スコットマイヤーズのこの記事で、これが危険な行為である理由とそれが引き起こす問題について詳しく読むことができます。構築中または破棄中に仮想関数を呼び出さないでください。「しかし、明らかに開発者はそれらが何をしているかを理解しており、エラーは発生していません。



また、「マイクロ最適化」セクションから11のV803診断中レベル警告があります。以前のイテレータ値が使用されていない場合。後置インクリメントの場合、追加の一時オブジェクトが作成されます。もちろん、これはエラーではなく、ほんの小さな詳細です。そのような診断が面白くない場合は、アナライザを使用するときに単純にオフにできます。まあ、原則として、「マイクロ最適化されており、デフォルトでオフになっています。



実際、記事でイテレータの増分などのささいなことを分析しているので、すべてが問題なく、何を補うべきかわからないことを理解していると思います。



非常に多くの場合、一部の診断はユーザーに適用されないか、興味がない可能性があります。サボテンを食べるのではなく、アナライザーの設定に少し時間を費やす方がよいでしょう。最も興味深いアナライザートリガーにすぐに近づくために実行する必要がある手順の詳細については、「CおよびC ++コードのPVS-Studioアナライザーによって発行された興味深い警告をすばやく表示するにはどうすればよいですか？」



セクションのトリガーの中でマイクロ最適化「興味深いV802診断警告もあります、構造体フィールドをタイプサイズの降順に配置することをお勧めします。これにより、構造体のサイズを小さくすることができます。



V802 64ビットプラットフォームでは、サイズに応じてフィールドを降順に並べ替えることにより、構造体のサイズを24バイトから16バイトに減らすことができます。HierarchicalClustering.h 31 



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


MaxClustersDistance フィールドをdoubleタイプおよびDistanceType列挙子と単純に交換することにより、構造体のサイズを24バイトから16バイトに減らすことができます。 



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


TDistanceFuncは列挙型であるため、そのサイズはint以下であるので、構造体の最後に移動する必要があります。



繰り返しますが、これは間違いではありませんが、ユーザーがマイクロ最適化を行うことに関心がある場合、または原則としてプロジェクトにとって重要である場合、そのようなアナライザートリガーを使用すると、少なくともプライマリリファクタリングの場所をすばやく見つけることができます。



一般に、すべてのコードはきちんと読みやすい形で記述されていますが、V807の診断では、より最適で読みやすいようにいくつかの場所が指摘されていました。最も印象的な例を挙げましょう：



V807パフォーマンスの低下。同じ式を繰り返し使用しないように、参照を作成することを検討してください。 GradientBoostFullTreeBuilder.cpp 469



image3.png


呼び出しcurLevelStatisticsは、[I] - > ThreadStatistics [j]は、別の変数への呼び出しに置き換えることができます。このチェーンでは複雑なメソッドの呼び出しはないため、特別な最適化は行われない可能性がありますが、私の意見では、コードの方がはるかに読みやすく、コンパクトに見えます。さらに、このコードのサポートにより、これらのインデックスに正確にアクセスする必要があり、ここでエラーが発生しないことが将来明らかになります。明確にするために、変数の代わりのコードを示します。 



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;
  }
}


結論



ご覧のとおり、静的分析に関しては、このフレームワークのコードベースは非常にクリーンであることがわかりました。



アクティブに開発されたプロジェクトでの1回の分析は、静的分析の必要性をほとんど反映していません。多くのエラーは、特に重要な場合、すでに他の方法で検出されていますが、時間とリソースを多く消費します。この点については、「静的コード分析では使用されないため、静的コード分析で検出できないエラー」という記事で詳しく分析されています。



しかし、この事実を念頭に置いても、NeoMLで警告はほとんど出されなかったので、開発者が静的分析を使用したかどうかに関係なく、このプロジェクトのコードの品質に対する敬意を表したいと思います。





この記事を英語を話す視聴者と共有したい場合は、翻訳リンクを使用してください：Victoria Khanieva。ABBYY NeoMLのコード品質に感銘を受けたPVS-Studio。


More articles:


All Articles