Пиши, сокращай, оптимизируй

• Заглянем в ассемблерный код
• Альтернативный подход
• Замер скорости работы
• Заключение

Попалась функция на языке С++. На её примере прям просится показать, что, делая рефакторинг, можно не только эстетично сократить код, но и оптимизировать его. Давайте разомнём мозги, они нам ещё пригодятся, несмотря на эпоху вайб-кодинга. Кто-то ведь должен понимать, как делать надо, а как не надо.

Приведённый ниже код я встретил в вайб-код проекте VibeTensor. Я исследую подобные проекты в качестве натуралиста (1, 2). Мне интересен генезис новых видов дефектов и недостатков в коде. Это позволит лучше понимать и описывать современные проблемы качества кода в эпоху генеративного AI (GenAI).

Одно из наблюдений – генерированный код более "пухлый", что затрудняет его восприятие человеком и оптимизацию компиляторами. Следующий фрагмент кода как раз это хорошо демонстрирует.

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  const std::size_t nd = sizes.size();
  std::vector<int64_t> strides(nd, 0);
  int64_t acc = 1;
  for (std::ptrdiff_t i = static_cast<std::ptrdiff_t>(nd) - 1; i >= 0; --i) {
    strides[static_cast<std::size_t>(i)] = acc;
    const auto sz = sizes[static_cast<std::size_t>(i)];
    acc *= (sz == 0 ? 1 : sz);
  }

  int64_t ne = 1;
  bool any_zero = false;
  for (auto s : sizes) {
    if (s == 0) {
      any_zero = true;
      break;
    }
    ne *= s;
  }
  if (any_zero) {
    ne = 0;
  }
  ....
}

С одной стороны, размер и скорость этого кода некритичны, так как он относится к тестам.

Однако этот код размножен по 9 файлам. Оставим за скобками, что такого по-хорошему вообще быть не должно. Но раз код размножается почкованием, желательно чтобы он был бы тогда по возможности компактным.

А ещё важно, что генерируется С++ код, и он обязан быть оптимальным. Само назначение языка – высокоэффективные приложения. В рассматриваемом месте – это просто медленный тест. Но в другом месте что-то подобное приведёт к существенному замедлению приложения. Или скажется кумулятивный эффект множества неудачных фрагментов сгенерированного кода.

Медленный С++ код – это противоестественно. Поэтому по-прежнему полезно развивать свою экспертность в понимании, удачным ли получился код и как его можно улучшить, сократить, оптимизировать. Или сгенерировать снова, используя уточнения.

В общем давайте потренируемся и проведём рефакторинг. Начнём с этого фрагмента:

int64_t ne = 1;
bool any_zero = false;
for (auto s : sizes) {
  if (s == 0) {
    any_zero = true;
    break;
  }
  ne *= s;
}
if (any_zero) {
  ne = 0;
}

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

Чтобы обнулить переменную, где хранится произведение, используется флаг any_zero. Этот подход избыточен, можно проще.

int64_t ne = 1;
for (auto s : sizes) {
  if (s == 0) {
    ne = 0;
    break;
  }
  ne *= s;
}

Когда встретится 0, обнулятся ne и цикл завершится. Можно продолжить упрощение. Современные процессоры быстро выполняют операции умножения. Поэтому можно в начале перемножить, а потом уже проверить.

int64_t ne = 1;
for (auto s : sizes) {
  ne *= s;
  if (s == 0) {
    break;
  }
}

Функциональность кода не изменилась, но он стал короче и, на мой взгляд, даже понятнее.

Если заранее знать, что всегда обрабатывается только несколько элементов или что тут точно не требуется какая-то скорость (вызывается в редкой ситуации), то код можно упростить до:

int64_t ne = 1;
for (auto s : sizes) {
  ne *= s;
}

Мы не будем думать про это, так как впоследствии рассмотренный цикл вообще будет "поглощён" другим.

Теперь вернёмся к началу функции.

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  const std::size_t nd = sizes.size();
  std::vector<int64_t> strides(nd, 0);
  int64_t acc = 1;
  for (std::ptrdiff_t i = static_cast<std::ptrdiff_t>(nd) - 1; i >= 0; --i) {
    strides[static_cast<std::size_t>(i)] = acc;
    const auto sz = sizes[static_cast<std::size_t>(i)];
    acc *= (sz == 0 ? 1 : sz);
  }

Из-за static_cast код выглядит тяжеловесным. Первое приведение типа нужно, чтобы какие-то компиляторы/статические анализаторы не ругались на странные арифметические игры.

std::ptrdiff_t i = static_cast<std::ptrdiff_t>(nd) - 1;

Рассмотрим, что будет, если убрать static_cast, а входной массив окажется пустым:

• Если массив пуст, то nd = 0;
• Вычтя единицу из беззнакового нуля, мы получим SIZE_MAX, т.е. очень большое положительное беззнаковое число.
• Значение SIZE_MAX типа size_t неявно преобразуется в тип ptrdiff_t и записывается в переменную i. Получается, что i = -1, как и было задумано. Но вот тут как раз могут быть выданы предупреждения. Ведь мы инициализируем ptrdiff_t числом, которое больше диапазона максимально вмещаемого числа. Поведение в такой ситуации до C++20 определяется реализацией – implementation defined behavior. После C++20 поведение определено.

Итого: первый static_cast лучше оставить на месте. Про остальные такого сказать нельзя. В них нет никакого смысла.

Оператор [] в классе vector принимает аргумент типа size_type (этот тип является синонимом size_t). Значение переменной i автоматически будет преобразовано в size_t, и в этом нет чего-то странного, опасного или подозрительного. Явное приведение типов только загромождает код, и от него лучше избавиться.

for (std::ptrdiff_t i = static_cast<std::ptrdiff_t>(nd) - 1; i >= 0; --i) {
  strides[i] = acc;
  const auto sz = sizes[i];
  acc *= (sz == 0 ? 1 : sz);
}

Первый шаг сделан. Можно теперь всё-таки упростить длинную строку с циклом? Давайте подумаем. Хочется написать как-то так:

for (auto sz : std::ranges::views::reverse(sizes)) {
  strides[???? i ????] = acc;
  acc *= (sz == 0 ? 1 : sz);
}

Всё равно требуется переменная i для обхода массива strides, начиная с конца. Так что совсем упростить код и избавиться от i не получается. Поэтому сделаем так:

int64_t acc = 1;
size_t i = nd;
for (auto sz : std::ranges::views::reverse(sizes)) {
  strides[--i] = acc;
  acc *= (sz == 0 ? 1 : sz);
}

Если честно, мне не нравится, что из-за использования --i код стал сложнее. Теперь требуется вникнуть, почему переменная в начале уменьшается, а уже затем используется для обращения к элементу массива.

С другой стороны, кода стало меньше и его можно быстрее просмотреть глазами. Так что, наверное, когнитивная сложность кода осталась в итоге такой же. Т.е. понимать код стало не проще, но и не сложнее. В любом случае код стал покороче, так что ok.

Взглянем целиком на вариант кода, который получился.

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  const std::size_t nd = sizes.size();
  std::vector<int64_t> strides(nd, 0);
  int64_t acc = 1;
  size_t i = nd;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--i] = acc;
    acc *= (sz == 0 ? 1 : sz);
  }

  int64_t ne = 1;

  for (auto s : sizes) {
    ne *= s;
    if (s == 0) {
      break;
    }
  }
  ....
}

Теперь, когда кода меньше, становится очевидным, что второй цикл избыточен. В первом цикле мы перебираем все элементы. Так почему бы их сразу не перемножить?

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  const std::size_t nd = sizes.size();
  std::vector<int64_t> strides(nd, 0);
  int64_t acc = 1;
  size_t i = nd;
  int64_t ne = 1;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--i] = acc;
    acc *= (sz == 0 ? 1 : sz);
    ne *= sz;
  }
  ....
}

Красота. Мы перемножаем все элементы, несмотря на то, что один из них может оказаться нулевым? Нестрашно. Микропроцессоры сейчас быстро умножают. Можно потерять больше на повторном доступе ко всем элементам во втором цикле.

Что ещё осталось? Не требуется изначально обнулять контейнер strides нулями. Всё равно все его элементы будут перезаписаны.

std::vector<int64_t> strides(nd, 0); // надо убрать второй аргумент

В принципе, мы закончили. Но можно сделать ещё одно косметическое изменение, избавившись от переменной nd. Она ни здесь, ни в последующем коде не нужна. Итоговый код:

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  auto q = sizes.size();
  std::vector<int64_t> strides(q);
  int64_t acc = 1;
  int64_t ne = 1;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--q] = acc;
    acc *= (sz == 0 ? 1 : sz);
    ne *= sz;
  }
  ....
}

Код сократился в два раза: с 20 до 9 строк!

Заглянем в ассемблерный код

Хорошо, текст функции стал короче, но скажется ли это положительно на производительности? Да. Посмотрим на основной фрагмент ассемблерного кода для изначального варианта функции. Используется Clang с ключом -O2.

.LBB1_7:
        mov     rax, rsi
        mov     qword ptr [rbx + 8*r12 - 16], rsi
        mov     rsi, qword ptr [r13 + 8*r12 - 16]
        cmp     rsi, 1
        adc     rsi, 0
        imul    rsi, rax
        dec     r12
        cmp     r12, 1
        ja      .LBB1_7
        mov     r12d, 1
        cmp     rbp, r13
        je      .LBB1_9
.LBB1_4:
        mov     rax, qword ptr [r13]
        test    rax, rax
        je      .LBB1_5
        imul    r12, rax
        add     r13, 8
        cmp     r13, rbp
        jne     .LBB1_4
        jmp     .LBB1_9

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

.LBB1_10:
        mov     rcx, rsi
        mov     rdx, qword ptr [rbp - 8]
        add     rbp, -8
        mov     qword ptr [rax], rsi
        cmp     rdx, 1
        mov     rsi, rdx
        adc     rsi, 0
        imul    rsi, rcx
        imul    r12, rdx
        add     rax, -8
        cmp     rbp, r13
        jne     .LBB1_10

Красивое. Один цикл. Коротко и быстро.

Тем, кто не очень хорошо понимает ассемблер, возможно покажется странным, что нет условного перехода для организации вот этого выбора: sz == 0 ? 1 : sz.

Альтернативный подход

Есть какой-то другой альтернативный подход? Есть. Не знаю, догадался ли я до него сам или нет. К сожалению, после написания своего варианта кода, я поспешил поспрашивать, какие варианты могут предложить ИИ.

Варианты от DeepSeek были не лучше, а некоторые даже хуже исходного. Например, он сократил одну из реализаций кода ценой переворачивания массива.

// Разворачиваем strides обратно
std::reverse(strides.begin(), strides.end());

А вот одна из альтернативных реализаций от Claude Opus заслуживает внимания. Он обратил внимание, что можно выполнять не две, а одну последовательность умножений!

Если все элементы входного массива не нулевые, то по завершению цикла acc == ne. А если хотя бы один элемент был нулевой, то в конце можно просто поменять значение ne на ноль. Происходит вновь возвращение к флагу any_zero, но в более умном варианте. Используя эту идею, можно написать следующий код:

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  auto q = sizes.size();
  std::vector<int64_t> strides(q);
  int64_t acc = 1;
  bool any_zero = false;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--q] = acc;
    acc *= (sz == 0 ? 1 : sz);
    any_zero |= sz == 0;
  }
  const int64_t ne = any_zero ? 0 : acc;
  ....
}

Будет ли код, в котором выполняется в два раза меньше умножений, более быстрым? Моё предсказание – необязательно. Процессор может выполнить два невзаимосвязанных умножения одновременно на разных конвейерах. Я делаю ставку, что мой вариант и вариант от Claude будут работать с практически одинаковой скоростью.

Замер скорости работы

Давайте проверим замерами, как оно будет на самом деле в реальности. Пришлось немного заморочиться с тестами. Во-первых, надо было подавать такие данные, чтобы не возникало переполнение знаковых 64-битных переменных, так как это UB и замерам не будет доверия. Во-вторых, для честности, надо чередовать данные с нулём и без. В-третьих, цепочки входных данных должны быть разной длины.

Не утверждаю, что измерения выверены, но общее представление они дают. Скорость работы первого изначального алгоритма взята за единицу отсчёта. Использовал Clang с ключом -O2 и -m64.

• Изначальный вариант c двумя циклами: 1.
• Мой вариант одним циклом: 0.88.
• Вариант, подсказанный Claude, с одним умножением: 0.92.

Рефакторинг дал ускорение в среднем на 10 %. Я ждал, что будет побольше, в районе 20—30 %. Но 10 % — это тоже хорошо, учитывая, что они достигнуты упрощением, а не усложнением кода.

Заключение

На этом разбор кода и его рефакторинг закончен. Был ли в этом смысл?

С точки зрения улучшения конкретно этого кода – нет. Это сгенерированные тесты. Некритично, что код длиннее и медленнее, чем он может быть.

С образовательной точки зрения – да. Неважно, создаётся код человеком или генерируется с помощью ИИ. Нужны эксперты, которые могут отличить хороший код от плохого. Быстрый он или нет. Безопасный он или нет.

Кому-то достаточно, что код работает, и неважно, что там. В некоторых случаях, например для прототипа, это нормально. Однако те, кто не интересуется архитектурой, оптимизацией, безопасностью, скорее всего, просто не видят и не понимают картину разработки и сопровождения больших программных продуктов. Нас ещё ждут новости о маленьких и больших провалах таких проектов.

ИИ — это мощный инструмент, но не волшебная палочка. Когда Claude подсказал мне альтернативный вариант алгоритма, это хороший пример, что ИИ можно использовать для исследований и усиления возможностей человека. Но точно также он легко нанесёт вред при бездумном использовании.

Если ты хочешь продолжать совершенствоваться как C++ специалист и создавать надёжное безопасное ПО, предлагаю заглянуть к нам сюда:

• Пишем на С++ компилятор для C++ подобного языка (10 уроков – 6,6 часов полезного видео).
• Разработка безопасного программного обеспечения (30 вебинаров – 50 часов, дополнительные материалы).
• Книга "Путеводитель C++ программиста по неопределённому поведению".
• Книга "Вредные советы для C++ программистов".