Pull to refresh

Comments 15

Спасибо. Тогда бы надо пройтись по всем, чтобы была сводная таблица


Любителям экономить на спичках могу еще посоветовать явно указывать компилятору на маловероятные ветки в условиях. Например, в GCC для этого есть __builtin_expect:

if (__builtin_expect(smth == NULL, 0))
    return EINVAL;

if (__builtin_expect(user_id != owner_id, 0))
    return EACCES;

return 0;

Приведенный выше код GCC скомпилирует как:

if (smth != NULL)
{
    if (user_id == owner_id)
    {
        return 0;
    }
    return EACCES;
}
return EINVAL;

В приведенных вами фрагментах кода очень много таких мест, где условия идут в «неверном» порядке.
Макрос:

#if defined(__GNUC__) && (__GNUC__ > 2)
#define likely(expr) (__builtin_expect((expr), 1))
#define unlikely(expr) (__builtin_expect((expr), 0))
#else
#define likely(expr) (expr)
#define unlikely(expr) (expr)
#endif

В коде:

if (unlikely(smth == NULL)) { /* ... */ }

Осторожнее с диагностикой V813 — замена значения на константную ссылку может поломать функцию если ее реализация где-то полагается на тот факт, что разные переменные указывают на разные участки памяти.

Интересное замечание, приведите, пожалуйста, пример подобного кода, хотя бы синтетический ибо по моим соображениям описанная вами ситуация может возникнуть только в некорректно написанном коде в целом, например, если код был портирован с C на С++ и в функциях ручное копирование данных в новое место было переделано некорректно.

Например:
В результате портирования появилась реализация в которой было убрано явное копирование данных и написали так:

void foo(type o)
{
    ...


Вместо того чтобы написать так:

void foo(const type& o)
{
     string copy_of_o = o;
...


А далее в коде используется сравнение адресов на члены класса, ну или возможно используется this как id объекта, что в целом не очень хорошая затея и часто указывает на плохой дизайн.

P.S. всё же прошу пример ибо подозреваю, что неправильно понял вами написанное, но ошибка кажется интересной :)

Ну вот например код с неоптимальной передачей параметров:


const uint32_t BASE = 1000000000;

void mul_add(std::vector<uint32_t> a, std::vector<uint32_t> b, std::vector<uint32_t>& c) {
    if (c.size() < a.size() + b.size() - 1)
        c.resize(a.size() + b.size() - 1);

    bool carry_on_last_iter = false;
    for (size_t i = 0; i < a.size(); i++) {
        uint64_t s = 0, ai = a[i];
        for (size_t j = 0; j < b.size(); j++) {
            s += c[i + j] + ai * b[j];
            c[i + j] = s % BASE;
            s /= BASE;
        }
        carry_on_last_iter = false;
        if (s > 0) {
            if (c.size() <= i + b.size())
                c.push_back(s);
            else {
                c[i + b.size()] += s;
                carry_on_last_iter = true;
            }
        }       
    }

    if (carry_on_last_iter) {
        for (size_t i = a.size() + b.size() - 1; c[i] > BASE; i++) {
            c[i] -= BASE;
            if (c.size() <= i)
                c.push_back(1);
            else
                c[i + 1]++;
        }
    }
}

А что случится если вызвать mul_add(a, a, a), а потом попытаться избежать копирования объявив параметры a и b как const&?

А что случится если вызвать mul_add(a, a, a), а потом попытаться избежать копирования объявив параметры a и b как const&?

Ничего хорошего. Но опять же, тут вопрос архитектуры этого API и в общем то разработчик ССЗБ. Накладные расходы на копирование векторов всё же не маленькие. Кроме того объекты a и b по логике работы функции всё равно константы, а в параметрах const забыт. В общем не могу я назвать приведённый код хорошим. Если код часто используемый (горячий в профилировщике) то в функцию следовало бы добавить отладочные проверки:
void mul_add(const std::vector<uint32_t>& a, const std::vector<uint32_t>& b, std::vector<uint32_t>& c) {
    assert(a.this != c.this); // use mul_add_same
    assert(b.this != c.this);
    if (c.size() < a.size() + b.size() - 1)
        c.resize(a.size() + b.size() - 1);

и добавить обвёртки mul_add_same для возможности работы с одним и тем же вектором, а лучше вообще дописать реализации для работы с одним и тем же вектором, да это дольше по времени, зато код будет значительно быстрее.

Если же код не часто используемый или вообще крайне редко используемый (не горячий в профилировщике), то необходимо явно дать понять и написать что функция обязана работать с копией данных. Т.е. вот так:
void mul_add(const std::vector<uint32_t>& _a, const std::vector<uint32_t>& _b, std::vector<uint32_t>& c) {
    const auto a(_a), b(_b);
    if (c.size() < a.size() + b.size() - 1)
        c.resize(a.size() + b.size() - 1);

… делать так необходимо чтобы код в первую очередь был явно читаем и интерпретируем человеком.

Отладочные проверки это хорошо (я сам такие же в реальном коде ставил) — но я не случайно написал вызов mul_add(a, a, a) первым. В данном случае, предлагаемая незначительная оптимизация ломает уже написанный код.


Почему оптимизация незначительная? Да потому что копирование векторов обычно работает намного быстрее чем два вложенных цикла в теле функции!

Почему оптимизация незначительная? Да потому что копирование векторов обычно работает намного быстрее чем два вложенных цикла в теле функции!

Эм, прошу прощения я только не понял, а откуда возьмутся два вложенных цикла? Oo

Посмотрите внимательнее тело функции mul_add. Там есть два вложенных цикла.

по результатам диагностики V803 предлагаю переименовать язык C++ в ++C.

Sign up to leave a comment.