Recentemente, a ABBYY publicou o código-fonte para sua estrutura NeoML. Nos ofereceram a verificação desta biblioteca usando o PVS-Studio. Este é um projeto interessante do ponto de vista da análise, portanto não o deixamos de lado. Não demorará muito tempo para ler este artigo, pois o projeto é de alta qualidade :).
O código fonte do NeoML pode ser encontrado no GitHub . Essa estrutura é multiplataforma e foi projetada para implementar modelos de aprendizado de máquina. É usado pelos desenvolvedores da ABBYY para resolver problemas de visão computacional, processamento de linguagem natural, incluindo processamento de imagem, análise de documentos e assim por diante. Atualmente, linguagens de programação como C ++, Java, Objective-C são suportadas e o Python será adicionado em breve a esta lista. A principal linguagem na qual o próprio framework foi escrito é C ++.
Análise em execução
Foi muito fácil executar a análise nessa estrutura. Após gerar o projeto Visual Studio no CMake, lancei a análise PVS-Studio no Visual Studio para os projetos de interesse da solução, excluindo bibliotecas de terceiros. Além do próprio NeoML, a solução também incluiu bibliotecas da ABBYY como NeoOnnx e NeoMathEngine. Também os incluí na lista de projetos para os quais a análise foi iniciada.
Resultados da análise
Claro, eu realmente queria encontrar alguns erros terríveis, mas ... o código acabou sendo limpo o suficiente e os avisos foram recebidos, nada. É provável que a análise estática já tenha sido usada no desenvolvimento. Muitos dos avisos foram acionados pelo mesmo diagnóstico em seções similares do código.
Por exemplo, neste projeto, um método virtual é frequentemente chamado do construtor. Em geral, essa é uma abordagem perigosa. O diagnóstico V1053 responde a esses casos : A chamada da função virtual 'foo' no construtor / destruidor pode levar a resultados inesperados no tempo de execução . Um total de 10 desses avisos foi emitido. Leia mais sobre por que essa é uma prática perigosa e quais problemas ela causa neste artigo de Scott Myers "Nunca chamar funções virtuais durante a construção ou destruição . No entanto, parece que os desenvolvedores a entender o que eles estão fazendo e não há erros.
Há também 11 V803 de nível médio de diagnóstico advertências da seção “microoptimizations”. Este diagnóstico recomenda substituir o incremento postfix com o incremento de prefixo, se o valor do iterador anterior não for usado. No caso de um incremento do postfix, um objeto temporário extra será criado. Claro, isso não é um erro, apenas um pequeno detalhe . Se esse diagnóstico não for interessante, você pode simplesmente desativá-lo ao usar o analisador. Bem, em princípio, o “micro- otimizações "e desativado por padrão.
Na verdade, acho que você entende que, como chegamos a uma análise de insignificantes como o incremento do iterador no artigo, significa que está tudo bem e não sabemos o que compensar.
Muitas vezes, alguns diagnósticos podem ser inaplicáveis ou desinteressantes para o usuário, e é melhor não comer um cacto, mas gastar um pouco de tempo configurando o analisador. Você pode ler mais sobre as etapas a serem seguidas para se aproximar imediatamente das respostas mais interessantes do analisador em nosso artigo " Como ver rapidamente os avisos interessantes que o analisador PVS-Studio gera para o código C e C ++? "
Entre os gatilhos da seção " micro-otimizações 'também possuem avisos de diagnóstico V802 interessantes, que recomenda organizar os campos da estrutura em ordem decrescente dos tamanhos dos tipos, o que possibilita reduzir o tamanho da estrutura.
V802 Na plataforma de 64 bits, o tamanho da estrutura pode ser reduzido de 24 para 16 bytes, reorganizando os campos de acordo com seus tamanhos em ordem decrescente. HierarchicalClustering.h 31
struct CParam {
TDistanceFunc DistanceType;
double MaxClustersDistance;
int MinClustersCount;
};Simplesmente trocando o campo MaxClustersDistance pelo tipo double e o enumerador DistanceType , você pode reduzir o tamanho da estrutura de 24 para 16 bytes.
struct CParam {
double MaxClustersDistance;
int MinClustersCount;
TDistanceFunc DistanceType;
};TDistanceFunc é uma enumeração ; portanto, seu tamanho é equivalente a um tipo int ou menor; portanto, deve ser movido para o final da estrutura.
Novamente, isso não é um erro, mas se o usuário estiver interessado em microoptimizações ou se forem, em princípio, importantes para o projeto, esses gatilhos do analisador permitem encontrar rapidamente locais para pelo menos a refatoração primária.
Em geral, todo o código é escrito com precisão e legibilidade , mas o diagnóstico do V807 apontava para alguns lugares que podem ser melhorados e otimizados. Deixe-me dar o exemplo mais ilustrativo:
V807 Desempenho reduzido. Considere criar uma referência para evitar usar a mesma expressão repetidamente. GradientBoostFullTreeBuilder.cpp 469
A chamada para curLevelStatistics [i] -> ThreadStatistics [j] pode ser substituída por uma chamada para uma variável separada. Não há nenhuma chamada de métodos complexos nessa cadeia; portanto, pode não haver nenhuma otimização especial aqui, mas o código, na minha opinião, será muito mais fácil de ler e parecer mais compacto. Além disso, com o suporte desse código, será claramente visto no futuro que é necessário acessar exatamente esses índices e não há erro aqui. Para maior clareza, darei o código com uma substituição para uma variável:
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;
}
}Conclusão
Como você pode ver, em termos de análise estática, a base de código dessa estrutura acabou sendo muito limpa.
Deve-se entender que um lançamento de análise em um projeto desenvolvido ativamente reflete fracamente a necessidade de análise estática, uma vez que muitos erros, especialmente se eles foram críticos, já foram detectados de outras maneiras, mas muito mais demorados e com muitos recursos. Este ponto foi analisado em mais detalhes no artigo " Erros que a análise de código estático não encontra porque não é usada ".
Mas, mesmo com esse fato em mente, foram emitidos poucos avisos no NeoML, e quero expressar meu respeito pela qualidade do código neste projeto, independentemente de os desenvolvedores terem usado análise estática ou não.
Se você deseja compartilhar este artigo com um público que fala inglês, use o link da tradução: Victoria Khanieva. PVS-Studio Impressionado com a qualidade do código do ABBYY NeoML .