Говоря о диагностических возможностях PVS-Studio в наших статьях, мы обычно опускаем его рекомендации по использованию микрооптимизаций в коде C и C++. Это, конечно, не так важно, как диагностика, выявляющая ошибки, но и они представляют собой интересную тему для обсуждения.
Микрооптимизации
Данная статья продолжает серию статей об анализе исходного кода операционной системы Tizen. Размер проекта Tizen (включая сторонние библиотеки) составляет 72 500 000 строк кода на C и C++, что делает его прекрасным примером для демонстрации различных аспектов использования статического анализа кода.
В предыдущей статье 27 000 ошибок в операционной системе Tizen было показано, как оценить общее количество ошибок определенного типа, которые анализатор потенциально может найти в заданном проекте, и рассмотрены многочисленные фрагменты кода, демонстрирующие возможности анализатора в выявление различных моделей дефектов. Однако наш инструмент не только обнаруживает ошибки, но и предлагает небольшие исправления, чтобы сделать код быстрее.
PVS-Studio определенно не предназначен и не способен заменить инструменты профилирования программного обеспечения. Только динамические анализаторы могут обнаружить узкие места; статические анализаторы не знают, какие входные данные подаются программам и как часто выполняется тот или иной фрагмент кода. Именно поэтому мы говорим о «микро-оптимизациях», которые вообще не гарантируют прироста производительности.
Если мы не можем ожидать заметного прироста производительности от микрооптимизаций, то нужны ли они нам вообще? Да, мы делаем, и вот причины:
- Диагностика, связанная с микрооптимизацией, часто выявляет плохой код. Исправляя его, вы делаете его проще, понятнее и, как правило, короче.
- Микрооптимизации малопригодны в конфигурации Release, так как современные компиляторы очень хороши в оптимизации, но некоторые микрооптимизации могут иметь эффект, далекий от «микро» на отладочной версии, что может быть полезно.
В PVS-Studio пока мало диагностик по микрооптимизациям (см. диагностика V801-V820), но мы будем добавлять больше. Мы не много говорили об этой диагностике в наших предыдущих статьях, поэтому сейчас самое время сделать это, когда мы изучаем исходный код Tizen.
Посмотрим, какие диагностики предлагает PVS-Studio для микрооптимизаций.
Примеры предупреждений
Как я упоминал в предыдущей статье, я изучил 3,3% кода Tizen. Это позволяет мне предсказать, сколько предупреждений определенного типа PVS-Studio сгенерирует для всего проекта, умножив количество уже найденных проблем на 30.
Запомните этот множитель, 30, так как я буду использовать его в своих расчетах на протяжении всей статьи.
V801: Аргумент функции N лучше переопределить как ссылку
Функции, получающие «тяжелые» аргументы по значению, неэффективны. Эта диагностика запускается, когда аргументы являются постоянными и обязательно останутся неизменными в теле функции.
Пример из Тизен:
inline void setLogTag(const std::string tagName) {
m_tag = tagName;
}PVS-Studio: V801 Снижение производительности. Лучше переопределить первый аргумент функции как ссылку. Попробуйте заменить «const .. tagName» на «const .. &tagName». Регистратор.ч 110
Создается дополнительный объект tagName, что является дорогостоящей операцией. Этот код выполняет такие дорогостоящие операции, как выделение памяти и копирование данных, но на самом деле они не нужны. Самый простой способ избежать их — передать аргумент постоянной ссылкой:
inline void setLogTag(const std::string &tagName) {
m_tag = tagName;
}Этот код больше не выполняет выделение памяти и копирование строк.
Есть и другой способ. Вы можете удалить const и данные будут перемещены, а не скопированы:
inline void setLogTag(std::string tagName) {
m_tag = std::move(tagName);
}Это решение столь же эффективно, как и предыдущее.
Этот пример с std::string,, конечно, безвреден. Наличие лишней строки в вашем коде не очень хорошо, но это никак не может повлиять на производительность. Однако могут быть и худшие случаи, такие как выделение массива строк. Об одном таком случае будет рассказано чуть позже, в разделе о диагностике V813.
Анализатор выдал 76 предупреждений для уже проверенных мною проектов.
Учтите, что анализатор иногда выдает ложноположительные предупреждения. Например, он может не отличить пользовательский интеллектуальный указатель от элемента односвязного списка. Оба являются указателями (на строку/следующий элемент). Оба имеют целочисленную переменную (длина строки/значение элемента списка). Они выглядят одинаково, но есть огромная разница в стоимости копирования этих объектов. Анализатор мог бы заглянуть в конструктор копирования и попытаться во всем этом разобраться, конечно, но это вообще безнадежно. По этой причине может быть хорошей идеей игнорировать такие предупреждения и подавлять их с помощью одного из механизмов подавления сообщений PVS-Studio. Возможно, позже я напишу об этом отдельную статью.
Помните множитель 30, о котором я упоминал ранее? Я могу использовать его для оценки общего количества предупреждений V801, которые PVS-Studio выдаст для всего проекта Tizen, и это число равно 76*30=2280.
V802: На 32-битной/64-битной платформе размер структуры можно уменьшить с N до K байт путем перестановки полей в соответствии с их размерами в порядке убывания.
Диагностика V802 ищет структуры и классы, размер которых можно уменьшить, отсортировав поля в порядке убывания размера. Вот пример неоптимальной структуры.
struct LiseElement {
bool m_isActive;
char *m_pNext;
int m_value;
};Эта структура будет занимать 24 байта памяти в 64-битной версии программы (LLP64) из-за выравнивания данных. Изменение порядка полей уменьшит его размер до 16 байт. Оптимизированная версия:
struct LiseElement {
char *m_pNext;
int m_value;
bool m_isActive;
};Обратите внимание, что размер этой структуры всегда равен 12 байтам в 32-битной версии, независимо от порядка полей. Вот почему 32-битная версия (ILP32LL) не вызывала предупреждение V802.
Еще одна вещь, которую вы должны знать, это то, что оптимизация структуры не всегда возможна или необходима.
Это невозможно, когда вам нужно сохранить совместимость формата данных. В еще большем количестве случаев в оптимизации просто нет необходимости. Если неоптимальные объекты создаются десятками или сотнями, заметного прироста производительности вы не получите. Оптимизация имеет смысл только при наличии миллионов элементов: в этом случае, чем меньше памяти занимает каждая структура, тем больше таких структур сможет хранить кэш ЦП.
Все сказанное выше говорит о том, что диагностика V802 имеет узкую область применения, поэтому в большинстве случаев ее желательно отключить, чтобы она не загромождала отчет анализа. В таком случае не вижу смысла оценивать общее количество неоптимальных структур, которые PVS-Studio смог найти в Tizen. Я думаю, что более 99% этих случаев можно было бы обойтись без оптимизации. Я лишь продемонстрирую, что такой анализ возможен всего на одном примере из Tizen.
typedef struct {
unsigned char format;
long long unsigned fields;
int index;
} bt_pbap_pull_vcard_parameters_t;PVS-Studio: V802 На 32-битной платформе размер структуры можно уменьшить с 24 до 16 байт, переставив поля в соответствии с их размерами в порядке убывания. bluetooth-api.h 1663
Если анализатор прав, тип long long unsigned должен быть выровнен по 8-байтовой границе при компиляции кода для платформы Tizen. Честно говоря, мы пока с этим не разобрались, так как эта платформа для нас новая, но так обстоят дела в известных мне системах :).
Итак, поскольку переменная fields выровнена по 8-байтовой границе, структура в памяти будет храниться следующим образом:
Члены класса можно переставить следующим образом:
typedef struct {
long long unsigned fields;
int index;
unsigned char format;
} bt_pbap_pull_vcard_parameters_t;Это решение поможет сэкономить 8 байт и структура будет держаться в памяти вот так:
Явно стало меньше.
V803. Более эффективно использовать префиксную форму ++it. Замените итератор++ на ++итератор
Книги по программированию рекомендуют использовать префикс, а не постфикс, приращение для итераторов цикла. Актуален ли этот совет, обсуждается в следующих статьях:
- Андрей Карпов. Разумно ли использовать префиксный оператор инкремента ++it вместо постфиксного оператора it++ для итераторов?.
- Сильвиу Арделеан. оператор приращения до и после — бенчмарк.
Короче говоря, это не имеет значения для версии Release; но это очень помогает в случае конфигурации отладки. Так что да, эта рекомендация по-прежнему уместна, и вы должны ей следовать. Обычно вы хотите, чтобы отладочная версия тоже была быстрой.
Пример предупреждения:
void ServiceManagerPrivate::loadServiceLibs()
{
auto end = servicesLoaderMap.end();
for(auto slm = servicesLoaderMap.begin(); slm !=end; slm++ ){
try{
ServiceFactory* factory=((*slm).second->getFactory());
servicesMap[factory->serviceName()] = factory;
}catch (std::runtime_error& e){
BROWSER_LOGD(e.what() );
}
}
}PVS-Studio: V803 Снижение производительности. В случае, если «slm» является итератором, более эффективно использовать префиксную форму приращения. Замените итератор++ на ++итератор. ServiceManager.cpp 67
Лучше заменить slm++ на ++slm. Одна замена, конечно, ничего не изменит — она сработает, только если вы применяете это систематически. В настоящее время в Tizen существует 103 проблемы такого типа, а это означает, что разработчикам придется оптимизировать в общей сложности около 3000 таких операций, если они захотят это сделать. Эти исправления сделают отладочную версию немного быстрее.
V804: функция Foo вызывается дважды в указанном выражении для вычисления длины одной и той же строки.
Иногда вы сталкиваетесь с кодом, который вычисляет длину заданной строки более одного раза. В отладочной конфигурации это однозначно означает падение производительности, особенно если такой код выполняется несколько раз. Как будет работать версия Release, неизвестно, но компилятор, скорее всего, не поймет, что он должен объединить несколько вызовов функции strlen в один вызов.
Посмотрите на следующий пример.
static isc_result_t
buildfilename(...., const char *directory, ....)
{
....
if (strlen(directory) > 0U &&
directory[strlen(directory) - 1] != '/')
isc_buffer_putstr(out, "/");
....
}PVS-Studio: V804 Снижение производительности. Функция strlen вызывается дважды в указанном выражении для вычисления длины одной и той же строки. dst_api.c 1832
Размер имени каталога вычисляется дважды. Кстати, этот код также вызывает предупреждение V805, но об этом мы поговорим в следующем разделе.
Этот код можно улучшить, добавив временную переменную для хранения длины строки:
const size_t directory_len = strlen(directory);
if (directory_len > 0U &&
directory[directory_len - 1] != '/')
isc_buffer_putstr(out, "/");Я не настаиваю на этом решении. Лично я считаю, что этот код и так достаточно хорош; Мне просто нужен был пример, чтобы объяснить диагностику. Тем не менее, исправление, не имеющее значения в этом конкретном случае, не означает, что оно бесполезно в любом другом случае: есть определенные циклы обработки строк, которые могут извлечь из этого пользу.
Код, который я проверил до сих пор, вызвал 20 предупреждений этого типа. Таким образом, общее количество, которое будет выпущено, составляет 600.
V805: Неэффективно идентифицировать пустую строку с помощью конструкции «strlen(str) › 0»
Вернемся к предыдущему примеру.
if (strlen(directory) > 0U &&
directory[strlen(directory) - 1] != '/')PVS-Studio: V805 Снижение производительности. Неэффективно идентифицировать пустую строку с помощью конструкции «strlen(str) › 0». Более эффективный способ — проверить: str[0] != ‘\0’. dst_api.c 1832
Помимо сохранения длины строки в промежуточную переменную, код можно оптимизировать еще одним способом. Первый вызов strlen необходим, чтобы проверить, пуста строка или нет, но вызов функции strlen для этой цели на самом деле является ненужной операцией, так как проверяется только первая строка. байт строки будет достаточно. Таким образом, код можно оптимизировать следующим образом:
if (*directory != '\0' &&
directory[strlen(directory) - 1] != '/')Или вот так:
if (directory[0] &&
directory[strlen(directory) - 1] != '/')И так далее. Существует множество способов реализации проверки. Форма на самом деле не имеет значения; важен тот факт, что вам не нужно просматривать каждый символ строки, чтобы узнать, пуста она или нет. Конечно, компилятор может понять замысел программиста и оптимизировать проверку в версии Release, но на такое везение рассчитывать не стоит.
Еще один пример:
V805 Снижение производительности. Неэффективно идентифицировать пустую строку с помощью конструкции «strlen(str) != 0». Более эффективный способ — проверить: str[0] != ‘\0’. bt-util.c 376
void _bt_util_set_phone_name(void) { char *phone_name = NULL; char *ptr = NULL;phone_name = vconf_get_str(VCONFKEY_SETAPPL_DEVICE_NAME_STR); if (!phone_name) return;if (strlen(phone_name) != 0) { // <= if (!g_utf8_validate(phone_name, -1, (const char **)&ptr)) *ptr = '\0';bt_adapter_set_name(phone_name); }free(phone_name); }
PVS-Studio: V805 Снижение производительности. Неэффективно идентифицировать пустую строку с помощью конструкции «strlen(str) != 0». Более эффективный способ — проверить: str[0] != ‘\0’. bt-util.c 376
В этом коде нет ничего особенного. Я просто хотел показать, что это типичный и очень распространенный способ проверки пустой строки. Я действительно удивлен тем, что в C нет стандартной функции или макроса для этой операции. Вы даже не представляете, сколько таких неэффективных проверок таится в наших программах. А теперь я расскажу вам одну ужасную вещь.
Часть кода Tizen, которую я уже проверил, содержит 415 случаев, когда функция strlen или ее аналог используется для проверки на наличие пустой строки.
Таким образом, предполагаемое количество предупреждений этого типа для всего проекта составляет 12450.
Только подумайте об этом огромном количестве бессмысленных циклов, которые ЦП должен повторять снова и снова в поисках конечного нуля, заполняя свои кэши данными, которые ему могут никогда не понадобиться!
Я считаю, что имеет смысл отказаться от таких неэффективных вызовов strlen. Возможные альтернативы:
- если (*имя_телефона)
- если (*имя_телефона != ‘\0’)
- если (имя_телефона[0])
- если (имя_телефона[0] != ‘\0’)
Однако мне эти реализации тоже не нравятся, потому что они недостаточно понятны. Гораздо лучше и понятнее сделать специальный макрос на языке C или встроенную функцию на языке C:
if (is_empty_str(phone_name))Как я уже сказал, мне кажется странным, что за все эти годы не было создано универсального стандартного средства проверки на наличие пустых C-строк. Если бы он был, это сделало бы огромные объемы кода немного быстрее. 12450 неэффективных проверок — это то, на что стоит обратить внимание, не так ли?
V806: выражение вида strlen(MyStr.c_str()) можно переписать как MyStr.length()
В отличие от предупреждений V805, которые были выданы в огромных количествах, до сих пор было всего два предупреждения V806 для проанализированного кода. Вот одна из тех редких птиц:
static void
panel_slot_forward_key_event (
int context, const KeyEvent &key, bool remote_mode)
{
....
if (strlen(key.get_key_string().c_str()) >= 116)
return;
....
}PVS-Studio: V806 Снижение производительности. Выражение вида strlen(MyStr.c_str()) можно переписать как MyStr.length(). wayland_panel_agent_module.cpp 2511
Такой код является типичным побочным эффектом рефакторинга старого кода C, превращенного в C++. Длина строки в переменной типа std::string вычисляется функцией strlen. Этот метод явно неэффективен и громоздок. Вот лучшее решение:
if (key.get_key_string().length() >= 116)
return;Код стал короче и быстрее. Ожидаемое общее количество предупреждений – 60.
V807: рассмотрите возможность создания указателя/ссылки, чтобы избежать многократного использования одного и того же выражения.
Иногда вы можете встретить выражения с большим количеством операторов «-›» и «.», как это:
To()->be.or->not().to()->be();В России это называется «поездное кодирование» (или «конгалайновое кодирование»). Я не знаю, есть ли английский термин для этого стиля программирования, но шаблон ясно объясняет метафору поезда.
Такой код считается плохим, и книги по качеству кода рекомендуют избегать его. Гораздо хуже ситуация, когда «поезда» повторяются много раз. Во-первых, они загромождают текст программы; во-вторых, они могут снизить производительность. Вот один из таких примеров:
PVS-Studio: V807 Снижение производительности. Рассмотрите возможность создания ссылки, чтобы избежать повторного использования одного и того же выражения. ImageObject.cpp 262
Этот код выглядел бы лучше, если бы он был переписан следующим образом:
for (....) {
auto &keypoint = obj.__features.__objectKeypoints[keypointNum];
os << keypoint.pt.x << ' ';
os << keypoint.pt.y << ' ';
os << keypoint.size << ' ';
os << keypoint.response << ' ';
os << keypoint.angle << ' ';
os << keypoint.octave << ' ';
os << keypoint.class_id << '\n';
}Было бы быстрее? Нет. Поскольку вставка потока — медленная операция, ускорение других операций не поможет, даже в конфигурации отладки.
Однако вторая версия короче, понятнее и удобнее в сопровождении.
Как я уже сказал, здесь мы не получим прироста производительности, но такая оптимизация может быть полезна в другом месте — например, когда «поезд» состоит из вызовов медленных, длительных функций. Компилятор может не понять, как оптимизировать такой код, и оставить вам массу ненужных вызовов функций.
На данный момент анализатор выдал 93 предупреждения такого типа. Прогнозируемое общее число составляет 2700.
V808: массив/объект был объявлен, но не использовался
Эта диагностика весьма интересна; он обнаруживает неиспользуемые переменные и массивы. Такие артефакты обычно появляются как побочный эффект плохого рефакторинга, когда программисты забывают удалить объявление переменной, которая больше не используется.
Неиспользуемые переменные тоже могут быть признаком логической ошибки, но такие случаи, насколько я знаю, нечасты.
Предупреждение выдается, когда:
- Массив создается, но не используется. Это означает, что функция потребляет больше памяти стека, чем необходимо. Во-первых, это может привести к переполнению стека; во-вторых, это может снизить производительность кэша процессора.
- Объекты класса создаются, но не используются. Анализатор предупреждает о таких объектах только тогда, когда их создание без дальнейшего использования — заведомо ненужная операция. Примерами этого являются std::string или CString. Создание и уничтожение этих объектов — пустая трата процессорного времени и стека.
Кстати, анализатор игнорирует лишние переменные типа float или char; в противном случае было бы слишком много ложных срабатываний. Эти переменные распространены в коде, который широко использует макросы или директивы препроцессора #if..#else..#endif. Эти дополнительные переменные безвредны, так как компилятор удалит их при оптимизации.
Давайте взглянем на пару предупреждений такого типа в Tizen:
void CynaraAdmin::userRemove(uid_t uid) { std::vector<CynaraAdminPolicy> policies; std::string user = std::to_string(static_cast<unsigned int>(uid));emptyBucket(Buckets.at(Bucket::PRIVACY_MANAGER),true, CYNARA_ADMIN_ANY, user, CYNARA_ADMIN_ANY); }
PVS-Studio: V808 создан объект «policies» типа «vector», но не использовался. cynara.cpp 499
Переменная policies не используется и должна быть удалена.
Следующий код более подозрительный:
static void _focused(int id, void *data, Evas_Object *obj, Elm_Object_Item *item) { struct menumgr *m = (struct menumgr *)data; Elm_Focus_Direction focus_dir[] = { ELM_FOCUS_LEFT, ELM_FOCUS_RIGHT, ELM_FOCUS_UP, ELM_FOCUS_DOWN }; int i; Evas_Object *neighbour;if (!obj || !m) return;if (m->info[id] && m->info[id]->focused) m->info[id]->focused(m->data, id);for (i = 0; i < sizeof(focus_dir) / sizeof(focus_dir[0]); ++i) { neighbour = elm_object_focus_next_object_get(obj, i); evas_object_stack_above(obj, neighbour); } }
PVS-Studio: V808 массив focus_dir был объявлен, но не использовался. менюmgr.c 110
Массив focus_dir не используется, что странно и может свидетельствовать о наличии какой-то ошибки, но это не обязательно так. Мне нужно более внимательно изучить этот код, чтобы сказать наверняка.
В настоящее время существует 30 предупреждений этого типа. Прогнозируемое число для всего проекта составляет 900.
V809: проверка «if (ptr != NULL)» может быть удалена.
Наконец-то мы добрались до диагностики, которая выдает больше всего предупреждений. Программисты обычно используют следующий шаблон кода:
if (P)
free(P);
if (Q)
delete Q;Это излишне. Только функция free и оператор delete могут достаточно хорошо обрабатывать нулевые указатели.
Код можно упростить:
free(P);
delete Q;Дополнительная проверка не делает его лучше, а только снижает его производительность.
Вероятно, вы могли бы возразить, что исходный код быстрее: если указатель нулевой, вам не нужно вводить функцию free или оператор delete и выполнять проверку внутри.
Я не согласен с этим. Большая часть кода написана на основе предположения, что указатели не равны нулю. Нулевой указатель, как правило, является редкой/опасной ситуацией, которая возникает нечасто. Поэтому почти каждый раз, когда мы вызываем free/delete, мы передаем ненулевой указатель. Предварительная проверка только вредит производительности и загромождает код.
Посмотрите на следующий пример:
lwres_freeaddrinfo(struct addrinfo *ai) { struct addrinfo *ai_next;while (ai != NULL) { ai_next = ai->ai_next; if (ai->ai_addr != NULL) free(ai->ai_addr); if (ai->ai_canonname) free(ai->ai_canonname); free(ai); ai = ai_next; } }
Здесь анализатор выдает сразу две лишние проверки:
- V809 Проверка того, что значение указателя не равно NULL, не требуется. Проверку if (ai-›ai_addr != NULL) можно убрать. getaddrinfo.c 694
- V809 Проверка того, что значение указателя не равно NULL, не требуется. Проверка «if (ai-›ai_canonname)» может быть удалена. getaddrinfo.c 696
Удалим лишние проверки:
lwres_freeaddrinfo(struct addrinfo *ai) { struct addrinfo *ai_next;while (ai != NULL) { ai_next = ai->ai_next; free(ai->ai_addr); free(ai->ai_canonname); free(ai); ai = ai_next; } }
Я нахожу эту версию намного проще и аккуратнее. Это просто прекрасный пример того, что такое рефакторинг.
Пока 620 предупреждений о лишних проверках такого типа!
Это означает, что вы должны ожидать около 18600 предупреждений для всего проекта Tizen! Вау! Только представьте себе — 18600 операторов if можно удалить без всякого риска!
V810: функция «А» вызывалась несколько раз с одинаковыми аргументами
#define TIZEN_USER_CONTENT_PATH tzplatform_getenv(TZ_USER_CONTENT)
int _media_content_rollback_path(const char *path, char *replace_path)
{
....
if (strncmp(path, TIZEN_USER_CONTENT_PATH,
strlen(TIZEN_USER_CONTENT_PATH)) == 0) {
....
}V810 Снижение производительности. Функция tzplatform_getenv(TZ_USER_CONTENT) вызывалась несколько раз с одинаковыми аргументами. Результат, возможно, следует сохранить во временную переменную, которую затем можно будет использовать при вызове функции «strncmp». media_util_private.c 328
Эта диагностика обнаруживает код, в котором функция вызывается с аргументами, представленными несколькими вызовами другой функции, каждый из которых имеет одинаковые аргументы. Если эти вызовы медленные, код можно оптимизировать, сохранив результат в промежуточной переменной.
В приведенном выше примере функция tzplatform_getenv вызывается дважды с одним и тем же аргументом.
Уже проверенная часть кода Tizen вызвала 7 предупреждений, и ни одно из них не выглядело достаточно интересным, поэтому никаких оценок.
V811: Чрезмерное приведение типов: string -> char * -> string
Эта диагностика выявляет неэффективные операции копирования строк, например:
std::string A = Foo();
std::string B(A.c_str());Строка B создается конструктором, который получает указатель на базовую строку с завершающим нулем. Перед выделением буфера программа должна вычислить длину строки A. Для этого ему придется пройти через всех своих персонажей. Это неэффективно, поскольку длину можно получить непосредственно из строки A. Код выглядел бы лучше, если бы он был переписан следующим образом:
std::string A = Foo();
std::string B(A);Эта реализация быстрее и короче.
Следующий пример взят из Tizen:
void PasswordUI::changeState(PasswordState state)
{
....
std::string text = "";
....
switch (m_state) {
case PasswordState::ConfirmPassword:
text = TabTranslations::instance().ConfirmPassword.c_str();
m_naviframe->setTitle("IDS_BR_HEADER_CONFIRM_PASSWORD_ABB2");
break;
....
}PVS-Studio: V811 Снижение производительности. Чрезмерное приведение типов: string -> char * -> string. Рассмотрите возможность проверки выражения. PasswordUI.cpp 242
На данный момент анализатор сообщил о 41 проблеме этого типа. Это означает, что ожидаемое общее количество неэффективных операций копирования строксоставляет1230.
V812: Неэффективное использование функции «счетчик»
Предупреждений V812 для Tizen не было, поэтому я просто кратко объясню, с какими типами дефектов связана эта диагностика.
Результат, возвращаемый стандартной библиотечной функцией count или count_if, сравнивается с нулем. Эта операция может быть медленной, поскольку этим функциям приходится сканировать весь контейнер, чтобы подсчитать количество необходимых элементов. Поскольку возвращаемое значение функции сравнивается с нулем, мы хотим знать, есть ли хотя бы один такой элемент. Более эффективный способ проверки наличия элемента-контейнера — использовать функцию find или find_if.
Медленный код:
void foo(const std::multiset<int> &ms)
{
if (ms.count(10) != 0) Foo();
}Быстрый код:
void foo(const std::multiset<int> &ms)
{
if (ms.find(10) != ms.end()) Foo();
}V813: Аргумент, вероятно, следует отображать как постоянный указатель/ссылку.
Аргумент, представленный структурой или классом, передается по значению. Анализатор проверяет тело функции и делает вывод, что аргумент не изменился. Это означает, что аргумент может быть передан как постоянная ссылка, что быстрее. Это может ускорить выполнение, поскольку при вызове функции будет скопирован только адрес, а не весь объект.
Диагностика V813 аналогична V801, за исключением того, что переменная не помечена как const. То есть анализатор должен сам разобраться, меняется переменная в функции или нет. Если это так, нет необходимости сообщать об этом. Возможны ложные срабатывания, но в целом эта диагностика работает безотказно.
Пример функции, запускающей эту диагностику в Tizen:
void
addDescriptions(std::vector<std::pair<int, std::string>> toAdd)
{
if (m_descCount + toAdd.size() > MAX_POLICY_DESCRIPTIONS) {
throw std::length_error("Descriptions count would exceed "
+ std::to_string(MAX_POLICY_DESCRIPTIONS));
}
auto addDesc = [] (DescrType **desc, int result,
const std::string &name)
{
(*desc) = static_cast<DescrType *>(malloc(sizeof(DescrType)));
(*desc)->result = result;
(*desc)->name = strdup(name.data());
};
for (const auto &it : toAdd) {
addDesc(m_policyDescs + m_descCount, it.first, it.second);
++m_descCount;
}
m_policyDescs[m_descCount] = nullptr;
}PVS-Studio: V813 Снижение производительности. Аргумент toAdd, вероятно, следует отображать как постоянную ссылку. CyadCommandlineDispatcherTest.h 63
Массив типа std::vector‹std::pair‹int, std::string›› передается по значению. Копирование массива такого размера — довольно затратная операция, не так ли?
Кроме того, он используется только для чтения. Было бы гораздо лучше объявить функцию следующим образом:
void addDescriptions(
const std::vector<std::pair<int, std::string>> &toAdd)Конечно, большинство случаев не столь критичны. Например:
void TabService::errorPrint(std::string method) const
{
int error_code = bp_tab_adaptor_get_errorcode();
BROWSER_LOGE("%s error: %d (%s)", method.c_str(), error_code,
tools::capiWebError::tabErrorToString(error_code).c_str());
}PVS-Studio: V813 Снижение производительности. Аргумент «метод», вероятно, следует отображать как постоянную ссылку. TabService.cpp 67
Этот код создает только одну дополнительную строку. Ничего страшного, но программиста-перфекциониста это все равно огорчает.
Я получил 303 предупреждения по проанализированным проектам, поэтому оценка для всего проекта составляет 9090. Я уверен, что многие из них нуждаются в оптимизации.
V814: функция strlen вызывалась несколько раз внутри тела цикла.
Вы, должно быть, уже заметили, что микрооптимизация в основном предлагается для строк. Дело в том, что большая часть этих диагностик была создана по запросу одного из наших клиентов, которому в коде требовалась эффективная обработка строк. Следующая диагностика не исключение — она тоже работает со строками.
Он обнаруживает циклы с вызовами функции strlen(S) или ее аналога. Строка S не меняется, поэтому ее длину можно вычислить заранее.
Вот два примера сообщений, выдаваемых этой диагностикой. Пример 1.
#define SETTING_FONT_PRELOAD_FONT_PATH "/usr/share/fonts" static Eina_List *_get_available_font_list() { .... for (j = 0; j < fs->nfont; j++) { FcChar8 *family = NULL; FcChar8 *file = NULL; FcChar8 *lang = NULL; int id = 0;if (FcPatternGetString(fs->fonts[j], FC_FILE, 0, &file) == FcResultMatch) { int preload_path_len = strlen(SETTING_FONT_PRELOAD_FONT_PATH); .... }
PVS-Studio: V814 Снижение производительности. Функция strlen вызывалась несколько раз внутри тела цикла. настройка-display.c 1185
Длина строки «/usr/share/fonts» будет вычисляться столько раз, сколько раз будет повторяться цикл. Компилятор, вероятно, поймет, как оптимизировать этот код, но вы не можете быть в этом уверены. Кроме того, отладочная версия все равно будет работать медленнее, чем могла бы.
Чтобы улучшить этот код, вам просто нужно вырезать строку, вычисляющую длину строки, и вставить ее перед началом цикла.
Пример 2.
static void BN_fromhex(BIGNUM *b, const char *str) { static const char hexdigits[] = "0123456789abcdef"; unsigned char data[512]; unsigned int i; BIGNUM *out;RUNTIME_CHECK(strlen(str) < 1024U && strlen(str) % 2 == 0U); for (i = 0; i < strlen(str); i += 2) { const char *s; unsigned int high, low;s = strchr(hexdigits, tolower((unsigned char)str[i])); RUNTIME_CHECK(s != NULL); high = (unsigned int)(s - hexdigits);s = strchr(hexdigits, tolower((unsigned char)str[i + 1])); RUNTIME_CHECK(s != NULL); low = (unsigned int)(s - hexdigits);data[i/2] = (unsigned char)((high << 4) + low); } out = BN_bin2bn(data, strlen(str)/2, b); RUNTIME_CHECK(out != NULL); }
PVS-Studio: V814 Снижение производительности. Несколько раз выполнялись вызовы функции strlen при вычислении условия продолжения цикла. openssldh_link.c 620
Анализатору не нравится эта строка:
for (i = 0; i < strlen(str); i += 2) {Длина строки, переданной в качестве аргумента, будет вычисляться на каждой итерации цикла. Кошмар программиста-перфекциониста.
Примечание. Такой код обычно пишут программисты, которые ранее кодировали на Pascal (среда Delphi). В Паскале условие завершения цикла вычисляется только один раз, поэтому оно допустимо и широко используется там. Подробности см. в главе 18. Знания, которыми вы обладаете, работая с одним языком, не всегда применимы к другому языку).
Кстати, не полагайтесь на компилятор для его оптимизации. Указатель на строку приходит извне. Конечно, строку нельзя изменить внутри функции (поскольку она имеет тип const char *), но это не значит, что ее нельзя изменить снаружи. Функция strchr, например, может это сделать, так что лучше перестраховаться…
Некоторым из вас может показаться странным, что постоянная ссылка может быть изменена. Посмотрите на код ниже.
int value = 1;
void Foo() { value = 2; }
void Example(const int &A)
{
printf("%i\n", A);
Foo();
printf("%i\n", A);
}
int main()
{
Example(value);
return 0;
}Хотя аргумент A имеет тип const int &, программа сначала напечатает значение 1, а затем 2.
Вот и все. Видите ли, const — это часть интерфейса доступа, которая запрещает изменение переменной, но это не означает, что переменная вообще не может быть изменена.
Оптимизированный код:
static void BN_fromhex(BIGNUM *b, const char *str) { static const char hexdigits[] = "0123456789abcdef"; unsigned char data[512]; unsigned int i; BIGNUM *out; const size_t strLen = strlen(str);RUNTIME_CHECK(strLen < 1024U && strLen % 2 == 0U); for (i = 0; i < strLen; i += 2) { const char *s; unsigned int high, low;s = strchr(hexdigits, tolower((unsigned char)str[i])); RUNTIME_CHECK(s != NULL); high = (unsigned int)(s - hexdigits);s = strchr(hexdigits, tolower((unsigned char)str[i + 1])); RUNTIME_CHECK(s != NULL); low = (unsigned int)(s - hexdigits);data[i/2] = (unsigned char)((high << 4) + low); } out = BN_bin2bn(data, strLen / 2, b); RUNTIME_CHECK(out != NULL); }
Уже проанализированные проекты содержат 112 вызовов функции strlen в циклах, которые могут быть выполнены только один раз. Ожидаемое общее количество предупреждений – 3360.
Вам не терпится получить себе копию PVS-Studio и отправиться делать этот мир лучше? Мы все за! Получите демо-версию здесь.
V815: рассмотрите возможность замены выражения «AA» на «BB».
Классы строк реализуют эффективные средства для очистки строки или проверки пустой строки. Я имею в виду, что лучше переписать следующий код:
void f(const std::string &A, std::string &B)
{
if (A != "")
B = "";
}следующим образом:
void f(const std::string &A, std::string &B)
{
if (!A. empty())
B.clear();
}Удастся ли компилятору оптимизировать Release-версию и построить одинаковый бинарный код как для первой, так и для второй версии функции?
Я поигрался с компилятором, который был у меня под рукой, Visual C++ (Visual Studio 2015), и ему удалось собрать один и тот же код для обеих версий проверки на пустую строку, но не удалось оптимизировать первую версию очистки строки, поэтому вызов функции std::basic_string::assign все еще существовала в двоичном коде.
Это пример предупреждения от Tizen:
services::SharedBookmarkFolder
FoldersStorage::getFolder(unsigned int id)
{
BROWSER_LOGD("[%s:%d] ", __PRETTY_FUNCTION__, __LINE__);
std::string name = getFolderName(id);
....
if (name != "")
folder = std::make_shared<services::BookmarkFolder>(
id, name, count);
return folder;
}PVS-Studio: V815 Снижение производительности. Рассмотрите возможность замены выражения ‘name != “”’ на ‘!name.empty()’. ПапкиStorage.cpp 134
Это была медленная проверка пустой строки, но у меня также есть пример медленной очистки строки:
....
std::string buffer;
....
bool GpsNmeaSource::tryParse(string data)
{
....
buffer = "";
....
}PVS-Studio: V815 Снижение производительности. Рассмотрите возможность замены выражения «buffer = «»» на «buffer.clear()». gpsnmea.cpp 709
Конечно, этот диагноз весьма спорный. Некоторые программисты предпочитают использовать выражение (str == "") для проверки пустой строки и присваивания четким строкам. Они считают, что такой код понятнее. С этим не поспоришь, особенно после того, как мой эксперимент показал, что проверка (str == "") будет оптимизирована компилятором в конфигурации Release.
Вам решать, следует ли вам использовать эти микрооптимизации или нет. Если вы решите против них, просто отключите эту диагностику. Тем не менее, иногда это может быть полезно, так как это было запрошено нашим заказчиком, а значит, это нужно людям.
На данный момент PVS-Studio выдал 63 предупреждения такого типа. Если разработчики Tizen сочтут эти предупреждения достойными внимания, им следует ожидать их в общей сложности 1890.
V816: Более эффективно ловить исключение по ссылке, а не по значению.
Лучше ловить исключения по ссылке, чем по значению. Помимо ускорения кода, это также убережет вас от некоторых других ошибок, таких как нарезка. Мы не будем обсуждать это здесь, потому что вопросами, связанными со слайсингом, занимается диагностика V746.
Вот пример:
std::string make_message(const char *fmt, ...)
{
....
try {
p = new char[size];
} catch (std::bad_alloc) {
Logger::getInstance().log("Error while allocating memory!!");
return std::string();
}
....
}PVS-Studio: V816 Более эффективно ловить исключение по ссылке, а не по значению. LoggerTools.cpp 37
Лучше переписать эту строку следующим образом:
} catch (std::bad_alloc &) {Я получил 84 предупреждения по проверенному коду. По оценкам: всего около 2500 предупреждений.
V817: более эффективно искать символ «X», а не строку.
Эта диагностика имеет дело с неэффективным поиском одиночных символов в строках. Самый простой способ объяснить это — рассмотреть два примера. Первое:
void URIEntry::_uri_entry_editing_changed_user(void* data,
Evas_Object*, void*)
{
....
if ((entry.find("http://") == 0)
|| (entry.find("https://") == 0)
|| (entry.find(".") != std::string::npos)) { // <=
self->setDocIcon();
} else {
....
}PVS-Studio: V817 Более эффективно искать символ «.», а не строку. URIEntry.cpp 211
Точку лучше искать как символ, а не как подстроку:
|| (entry.find('.') != std::string::npos)) {Второй случай аналогичен:
char *_gl_info__detail_title_get(
void *data, Evas_Object *obj, const char *part)
{
....
p = strstr(szSerialNum, ",");
....
}PVS-Studio: V817 Более эффективно искать символ ',', а не строку. настройка-info.c 511
Запятую лучше искать с помощью функции strchr:
p = strchr(szSerialNum, ',');Уже проверенные мной проекты содержат 37 выпусков этого типа. Ожидаемое общее число: 1110.
Новая диагностика
На момент написания этой статьи в PVS-Studio 6.16 были добавлены новые диагностики: V818, V819, V820. Они еще не были готовы, когда я проверял Tizen, поэтому у меня нет оттуда примеров, чтобы показать вам. Перейдите по этим ссылкам, чтобы узнать, что они делают:
- В818. Более эффективно использовать список инициализации, а не оператор присваивания.
- В819. Снижение производительности. Память выделяется и освобождается несколько раз внутри тела цикла.
- В820. Переменная не используется после копирования. Копирование можно заменить перемещением/обменом для оптимизации.
Резюме
Надеюсь, вы многое узнали из этой статьи о наборе диагностик PVS-Studio, о котором мы почти никогда не упоминаем. Возможно, они помогут некоторым из вас улучшить свой код. Хотя в настоящее время они по большей части имеют дело со случаями неэффективной обработки строк (std::string, CString,и т. д.), мы собираемся включить диагностику для других не- оптимальные шаблоны кода в будущем.
Давайте посчитаем, сколько предупреждений нам следует ожидать, которые, на мой взгляд, могли бы помочь сделать Tizen и библиотеки лучше.
- V801–2280
- V803–3000
- V804–600
- V805–12450
- V806–60
- V807–2700
- V808–900
- V809–18600
- V811–1230
- V813–9090
- V814–3360
- V815–1890
- V816–2500
- V817–1110
ВСЕГО: около 59000 предупреждений
Я не говорю, что вы должны пойти и исправить каждое из этих предупреждений. Я знаю, что это не сделает Tizen заметно быстрее. Кроме того, применение такого количества исправлений создает риск сломать что-то из-за небольшой опечатки.
Тем не менее, я считаю, что эти предупреждения имеют смысл. Разумное обращение с ними может помочь вам создать более простой и эффективный код.
Если вы спросите меня, старый код следует оставить как есть, но новый код должен использовать эти микрооптимизации. Эта статья ясно показывает, что большую часть кода можно было бы улучшить, хотя бы немного.
Вывод
Добро пожаловать на установку PVS-Studio и попробуйте ее со своими проектами. Если вы используете Windows, демо-версия доступна сразу. Если вы хотите запустить его на Linux, пожалуйста, напишите нам, чтобы получить временный лицензионный ключ.
Спасибо за чтение!
дальнейшее чтение
- Андрей Карпов. 27 000 ошибок в операционной системе Tizen.
- Сергей Васильев. Как PVS-Studio может помочь в обнаружении уязвимостей?
- Андрей Карпов. Выдача лицензий PVS-Studio Analyzer специалистам по безопасности.
- Андрей Карпов. Команда PVS-Studio: аудит кода и другие услуги.
- Сергей Хренов. PVS-Studio как плагин для SonarQube.
- Екатерина Миловидова. Ошибка месяца: переход от PC-Lint к PVS-Studio.
