El otro día, ABBYY ha publicado el código fuente de su marco NeoML. Nos ofrecieron verificar esta biblioteca usando PVS-Studio. Este es un proyecto interesante desde el punto de vista del análisis, por lo que no lo posponemos en segundo plano. Leer este artículo no te llevará mucho tiempo, ya que el proyecto resultó ser de alta calidad :).
El código fuente de NeoML se puede encontrar en GitHub . Este marco es multiplataforma y está diseñado para implementar modelos de aprendizaje automático. Los desarrolladores de ABBYY lo utilizan para resolver problemas de visión por computadora, procesamiento del lenguaje natural, incluido el procesamiento de imágenes, análisis de documentos, etc. Actualmente, los lenguajes de programación como C ++, Java, Objective-C son compatibles, y Python debería agregarse a esta lista pronto. El lenguaje principal en el que se escribió el marco en sí es C ++.
Ejecutar análisis
Ejecutar el análisis en este marco fue muy simple. Después de generar un proyecto de Visual Studio en CMake, lancé el análisis PVS-Studio en Visual Studio para los proyectos que nos interesan desde Solution, excluyendo las bibliotecas de terceros. Además del propio NeoML, la solución también incluía bibliotecas de ABBYY como NeoOnnx y NeoMathEngine. También los incluí en la lista de proyectos para los que se lanzó el análisis.
Resultados de analisis
Por supuesto, realmente quería encontrar algunos errores terribles, pero ... el código resultó ser bastante limpio y solo se recibieron advertencias, nada. Es probable que el desarrollo ya haya utilizado el análisis estático. Muchas de las advertencias fueron activadas por los mismos diagnósticos en secciones similares de código.
Por ejemplo, en este proyecto es muy común llamar a un método virtual desde un constructor. En general, este es un enfoque peligroso. El diagnóstico V1053 responde a tales casos : llamar a la función virtual 'foo' en el constructor / destructor puede generar resultados inesperados en tiempo de ejecución . Se emitieron un total de 10 de tales advertencias. Lea más sobre por qué es una práctica peligrosa y qué problemas causa en este artículo de Scott Myers "Nunca llame a funciones virtuales durante la construcción o la destrucción ". Sin embargo, aparentemente los desarrolladores entienden lo que están haciendo, y no hay errores.
También hay 11 advertencias de diagnóstico V803 de la sección de micro-optimizaciones. Este diagnóstico recomienda reemplazar el incremento de postfijo con un prefijo. si no se usa el valor anterior del iterador. En el caso de un incremento de postfix, se crea un objeto temporal adicional. Por supuesto, esto no es un error, solo un pequeño detalle . Si tales diagnósticos no son interesantes, entonces cuando use el analizador, simplemente puede desactivarlo. Bueno, en principio, un conjunto de "micro- optimizaciones "y apagado por defecto.
En realidad, creo que entiendes que, dado que hemos analizado aspectos tan pequeños como el incremento del iterador en el artículo, significa que todo está bien y simplemente no sabemos qué compensar.
Muy a menudo, algunos diagnósticos pueden ser inaplicables o poco interesantes para el usuario, y es mejor no comer un cactus, sino pasar un poco de tiempo configurando el analizador. Puede leer más sobre los pasos que se deben seguir para acercarse de inmediato a los desencadenantes más interesantes del analizador en nuestro artículo " ¿Cómo ver rápidamente las advertencias interesantes emitidas por el analizador PVS-Studio para el código C y C ++? "
Entre los desencadenantes de la sección " las micro optimizaciones también tienen interesantes advertencias de diagnóstico V802, que recomienda organizar los campos de estructura en orden descendente de los tamaños de los tipos, lo que permite reducir el tamaño de la estructura.
V802 En la plataforma de 64 bits, el tamaño de la estructura se puede reducir de 24 a 16 bytes reorganizando los campos de acuerdo con sus tamaños en orden decreciente. HierarchicalClustering.h 31
struct CParam {
TDistanceFunc DistanceType;
double MaxClustersDistance;
int MinClustersCount;
};Simplemente intercambiando el campo MaxClustersDistance de tipo double y el enumerador DistanceType puede reducir el tamaño de la estructura de 24 a 16 bytes.
struct CParam {
double MaxClustersDistance;
int MinClustersCount;
TDistanceFunc DistanceType;
};TDistanceFunc es una enumeración , por lo que su tamaño es equivalente a int o menos, por lo que se debe mover al final de la estructura.
Nuevamente, esto no es un error, pero si el usuario está interesado en realizar micro optimizaciones o si, en principio, son importantes para el proyecto, estos activadores del analizador le permiten encontrar rápidamente lugares para al menos una refactorización primaria.
En general, todo el código está escrito de manera precisa y legible, pero los diagnósticos del V807 apuntan a un par de lugares que pueden hacerse un poco más óptimos y legibles. Déjame darte el ejemplo más ilustrativo:
V807 Disminución del rendimiento. Considere crear una referencia para evitar usar la misma expresión repetidamente. GradientBoostFullTreeBuilder.cpp 469
La llamada a curLevelStatistics [i] -> ThreadStatistics [j] se puede reemplazar con una llamada a una variable separada. No se requiere ningún método complejo en esta cadena, por lo que puede que no haya una optimización especial aquí, pero el código, en mi opinión, será mucho más fácil de leer y se verá más compacto. Además, con el soporte de este código, se verá claramente en el futuro que es necesario acceder exactamente a estos índices y que no hay ningún error aquí. Para mayor claridad, le daré el código con un reemplazo para una variable:
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;
}
}Conclusión
Como puede ver, en términos de análisis estático, la base de código de este marco resultó ser muy limpia.
Debe entenderse que una ejecución de análisis en un proyecto desarrollado activamente refleja débilmente la necesidad de un análisis estático, ya que muchos errores, especialmente si eran críticos, ya se han detectado de otras maneras, pero requieren mucho más tiempo y recursos. Este punto se analizó con más detalle en el artículo " Errores que el análisis de código estático no encuentra porque no se utiliza ".
Pero incluso con este hecho en mente, se emitieron pocas advertencias en NeoML, y quiero expresar respeto por la calidad del código en este proyecto, independientemente de si los desarrolladores usaron análisis estáticos o no.
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace de traducción: Victoria Khanieva. PVS-Studio Impresionado por la calidad del código de ABBYY NeoML .