[] PVS-Studio впечатлен качеством кода Abbyy NeoML
Автор
Сообщение
news_bot ®
Стаж: 6 лет 9 месяцев
Сообщений: 27286
На днях компания ABBYY опубликовала исходный код своего фреймворка NeoML. Нам предложили проверить эту библиотеку с помощью PVS-Studio. Это интересный проект с точки зрения анализа, так что мы не стали откладывать его в долгий ящик. Чтение этой статьи не займет у вас много времени, так как проект оказался высокого качества :).
Исходный код NeoML можно найти на GitHub. Этот фреймворк является кроссплатформенным и предназначен для реализации моделей машинного обучения. Он используется разработчиками компании ABBYY для решения задач компьютерного зрения, обработки естественного языка, в том числе обработки изображений, анализа документов и так далее. В настоящее время поддерживаются такие языки программирования как C++, Java, Objective-C, и скоро к этому списку должен добавиться Python. Основной язык, на котором был написан сам фреймворк, это С++.
Запуск анализа
Запустить анализ на этом фреймворке было очень просто. После генерации проекта Visual Studio в CMake я запустила анализ PVS-Studio в Visual Studio на интересующие нас проекты из Solution, исключая сторонние библиотеки. Помимо самого NeoML в решении присутствовали еще такие библиотеки от ABBYY как NeoOnnx и NeoMathEngine. Их я также включила в список проектов, для которых запускался анализ.
Результаты анализа
Конечно же очень хотелось найти какие-нибудь страшные ошибки, но… код оказался достаточно чистым и предупреждений было получено всего-ничего. Вполне вероятно, что при разработке уже использовался статический анализ. Многие из предупреждений были срабатываниями одних и тех же диагностик на схожие участки кода.
Например, в этом проекте очень часто происходит вызов виртуального метода из конструктора. В целом это опасный подход. На такие случаи реагирует диагностика V1053: Calling the 'foo' virtual function in the constructor/destructor may lead to unexpected result at runtime. В общей сложности было выдано 10 таких предупреждений. Подробнее о том, почему это опасная практика и какие проблемы она вызывает можно почитать в этой статье Скотта Майерса "Never Call Virtual Functions during Construction or Destruction". Однако, по всей видимости разработчики понимают, что они делают, и ошибок здесь нет.
Также есть 11 предупреждений среднего уровня диагностики V803 из раздела "микрооптимизаций". Эта диагностика рекомендует заменить постфиксный инкремент на префиксный, если предыдущее значение итератора не используется. В случае постфиксного инкремента создается лишний временный объект. Конечно же это не является ошибкой, просто маленькая деталь. Если такая диагностика неинтересна, то при использовании анализатора можно её попросту выключить. Ну и в принципе, набор "микро-оптимизаций" и выключен по умолчанию.
Собственно, думаю вы понимаете, что раз мы дошли до разбора в статье таких мелочей, как инкремент итератора, то значит вообще всё хорошо и мы просто не знаем к чему попридираться.
Очень часто некоторые диагностики могут быть неприменимы или неинтересны для пользователя и лучше не есть кактус, а потратить немного времени на настройку анализатора. Подробнее о шагах, которые стоит предпринять для того, чтобы сразу подобраться поближе к наиболее интересным срабатываниям анализатора, можно почитать в нашей статье "Как быстро посмотреть интересные предупреждения, которые выдает анализатор PVS-Studio для C и C++ кода?"
Среди срабатываний из раздела "микрооптимизаций" также есть интересные предупреждения диагностики V802, которая рекомендует расположить поля структуры по убыванию размеров типов, что позволяет снизить размер структуры.
V802 On 64-bit platform, structure size can be reduced from 24 to 16 bytes by rearranging the fields according to their sizes in decreasing order. HierarchicalClustering.h 31
struct CParam {
TDistanceFunc DistanceType;
double MaxClustersDistance;
int MinClustersCount;
};
Всего лишь поменяв местами поле MaxClustersDistance с типом double и поле MinClustersCount с типом int можно снизить размер структуры с 24 до 16 байт.
struct CParam {
TDistanceFunc DistanceType;
int MinClustersCount;
double MaxClustersDistance;
};
TDistanceFunc это enum, так что его размер эквивалентен int или меньшему типу, и мы можем оставить его на первой позиции.
Опять же это не ошибка, но если пользователю интересно заняться микрооптимизациями или если они в принципе важны для проекта, такие срабатывания анализатора позволяют быстро найти места для хотя бы первичного рефакторинга.
В целом весь код написан аккуратно и разборчиво, но диагностика V807 указала на пару мест, которые можно сделать чуть более оптимальными и читабельными. Приведу наиболее наглядный пример:
V807 Decreased performance. Consider creating a reference to avoid using the same expression repeatedly. GradientBoostFullTreeBuilder.cpp 469
Обращение к curLevelStatistics->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;
}
}
Заключение
Как видите, с точки зрения статического анализа, кодовая база этого фреймворка оказалось очень чистой.
Стоит понимать, что один запуск анализа на активно разрабатываемый проект слабо отражает необходимость в статическом анализе, так как многие ошибки, особенно если они были критическими, уже были обнаружены и другими путями, но куда более затратными по времени и ресурсам. Чуть подробнее этот момент был разобран в статье "Ошибки, которые не находит статический анализ кода, потому что он не используется".
Но даже с учетом этого факта, на NeoML было выдано мало предупреждений, и я хочу выразить уважение качеству кода в этом проекте, вне зависимости от того использовался ли разработчиками статический анализ или нет.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Victoria Khanieva. PVS-Studio Impressed by the Code Quality of ABBYY NeoML.
===========
Источник:
habr.com
===========
Похожие новости:
- [] PVS-Studio Impressed by the Code Quality of ABBYY NeoML
- [Git, GitHub, Open source, Краудсорсинг, Научная фантастика] Альтернативная конституция
- [Godot, Прототипирование, Разработка игр] Godot, 1000 мелочей
- [*nix, C, Open source, Звук, Разработка под Linux] Изучаем VoIP-движок Mediastreamer2. Часть 13, заключительная
- [NoSQL, Open source] Создатель СУБД Redis уходит от сопровождения проекта
- [IT-инфраструктура, Open source, Виртуализация, Разработка под Linux, Сетевые технологии] Интеграция Open vSwitch с Р-виртуализацией
- [Open source, Настройка Linux, Процессоры] Линус Торвальдс о будущем Linux: «Сложно найти мейнтейнеров»
- [Angular, JavaScript, Open source, TypeScript] Лабаем на MIDI клавиатуре в Angular
- [C#, Open source, Биотехнологии, Клиентская оптимизация, Научно-популярное] Играем в Бога, причиняем непрошеную помощь науке и немножечко Сингулярности
- [DevOps, Kubernetes, Open source, Программирование, Системное администрирование] Удаляем устаревшую feature branch в Kubernetes кластере
Теги для поиска: #_pvsstudio, #_machine_learning, #_open_source, #_static_analysis, #_blog_kompanii_pvsstudio (
Блог компании PVS-Studio
)
Вы не можете начинать темы
Вы не можете отвечать на сообщения
Вы не можете редактировать свои сообщения
Вы не можете удалять свои сообщения
Вы не можете голосовать в опросах
Вы не можете прикреплять файлы к сообщениям
Вы не можете скачивать файлы
Текущее время: 21-Ноя 17:27
Часовой пояс: UTC + 5
| Автор | Сообщение |
|---|---|
|
news_bot ®
Стаж: 6 лет 9 месяцев |
|
 На днях компания ABBYY опубликовала исходный код своего фреймворка NeoML. Нам предложили проверить эту библиотеку с помощью PVS-Studio. Это интересный проект с точки зрения анализа, так что мы не стали откладывать его в долгий ящик. Чтение этой статьи не займет у вас много времени, так как проект оказался высокого качества :). Исходный код NeoML можно найти на GitHub. Этот фреймворк является кроссплатформенным и предназначен для реализации моделей машинного обучения. Он используется разработчиками компании ABBYY для решения задач компьютерного зрения, обработки естественного языка, в том числе обработки изображений, анализа документов и так далее. В настоящее время поддерживаются такие языки программирования как C++, Java, Objective-C, и скоро к этому списку должен добавиться Python. Основной язык, на котором был написан сам фреймворк, это С++. Запуск анализа Запустить анализ на этом фреймворке было очень просто. После генерации проекта Visual Studio в CMake я запустила анализ PVS-Studio в Visual Studio на интересующие нас проекты из Solution, исключая сторонние библиотеки. Помимо самого NeoML в решении присутствовали еще такие библиотеки от ABBYY как NeoOnnx и NeoMathEngine. Их я также включила в список проектов, для которых запускался анализ. Результаты анализа Конечно же очень хотелось найти какие-нибудь страшные ошибки, но… код оказался достаточно чистым и предупреждений было получено всего-ничего. Вполне вероятно, что при разработке уже использовался статический анализ. Многие из предупреждений были срабатываниями одних и тех же диагностик на схожие участки кода. Например, в этом проекте очень часто происходит вызов виртуального метода из конструктора. В целом это опасный подход. На такие случаи реагирует диагностика V1053: Calling the 'foo' virtual function in the constructor/destructor may lead to unexpected result at runtime. В общей сложности было выдано 10 таких предупреждений. Подробнее о том, почему это опасная практика и какие проблемы она вызывает можно почитать в этой статье Скотта Майерса "Never Call Virtual Functions during Construction or Destruction". Однако, по всей видимости разработчики понимают, что они делают, и ошибок здесь нет. Также есть 11 предупреждений среднего уровня диагностики V803 из раздела "микрооптимизаций". Эта диагностика рекомендует заменить постфиксный инкремент на префиксный, если предыдущее значение итератора не используется. В случае постфиксного инкремента создается лишний временный объект. Конечно же это не является ошибкой, просто маленькая деталь. Если такая диагностика неинтересна, то при использовании анализатора можно её попросту выключить. Ну и в принципе, набор "микро-оптимизаций" и выключен по умолчанию. Собственно, думаю вы понимаете, что раз мы дошли до разбора в статье таких мелочей, как инкремент итератора, то значит вообще всё хорошо и мы просто не знаем к чему попридираться. Очень часто некоторые диагностики могут быть неприменимы или неинтересны для пользователя и лучше не есть кактус, а потратить немного времени на настройку анализатора. Подробнее о шагах, которые стоит предпринять для того, чтобы сразу подобраться поближе к наиболее интересным срабатываниям анализатора, можно почитать в нашей статье "Как быстро посмотреть интересные предупреждения, которые выдает анализатор PVS-Studio для C и C++ кода?" Среди срабатываний из раздела "микрооптимизаций" также есть интересные предупреждения диагностики V802, которая рекомендует расположить поля структуры по убыванию размеров типов, что позволяет снизить размер структуры. V802 On 64-bit platform, structure size can be reduced from 24 to 16 bytes by rearranging the fields according to their sizes in decreasing order. HierarchicalClustering.h 31 struct CParam {
TDistanceFunc DistanceType; double MaxClustersDistance; int MinClustersCount; }; Всего лишь поменяв местами поле MaxClustersDistance с типом double и поле MinClustersCount с типом int можно снизить размер структуры с 24 до 16 байт. struct CParam {
TDistanceFunc DistanceType; int MinClustersCount; double MaxClustersDistance; }; TDistanceFunc это enum, так что его размер эквивалентен int или меньшему типу, и мы можем оставить его на первой позиции. Опять же это не ошибка, но если пользователю интересно заняться микрооптимизациями или если они в принципе важны для проекта, такие срабатывания анализатора позволяют быстро найти места для хотя бы первичного рефакторинга. В целом весь код написан аккуратно и разборчиво, но диагностика V807 указала на пару мест, которые можно сделать чуть более оптимальными и читабельными. Приведу наиболее наглядный пример: V807 Decreased performance. Consider creating a reference to avoid using the same expression repeatedly. GradientBoostFullTreeBuilder.cpp 469  Обращение к curLevelStatistics->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; } } Заключение Как видите, с точки зрения статического анализа, кодовая база этого фреймворка оказалось очень чистой. Стоит понимать, что один запуск анализа на активно разрабатываемый проект слабо отражает необходимость в статическом анализе, так как многие ошибки, особенно если они были критическими, уже были обнаружены и другими путями, но куда более затратными по времени и ресурсам. Чуть подробнее этот момент был разобран в статье "Ошибки, которые не находит статический анализ кода, потому что он не используется". Но даже с учетом этого факта, на NeoML было выдано мало предупреждений, и я хочу выразить уважение качеству кода в этом проекте, вне зависимости от того использовался ли разработчиками статический анализ или нет.  Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Victoria Khanieva. PVS-Studio Impressed by the Code Quality of ABBYY NeoML. =========== Источник: habr.com =========== Похожие новости:
Блог компании PVS-Studio ) |
|
Вы не можете начинать темы
Вы не можете отвечать на сообщения
Вы не можете редактировать свои сообщения
Вы не можете удалять свои сообщения
Вы не можете голосовать в опросах
Вы не можете прикреплять файлы к сообщениям
Вы не можете скачивать файлы
Вы не можете отвечать на сообщения
Вы не можете редактировать свои сообщения
Вы не можете удалять свои сообщения
Вы не можете голосовать в опросах
Вы не можете прикреплять файлы к сообщениям
Вы не можете скачивать файлы
Текущее время: 21-Ноя 17:27
Часовой пояс: UTC + 5