Skip to content

Conversation

@DanielYouCan
Copy link

No description provided.

Copy link
Owner

@spajic spajic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Аппрув + несколько комментов.
Респект за assert_performance_linear!

RSS для целевого файла колеблется в пределах 2.6-2.9 Gb.

## Защита от регресса производительности
Для защиты от потери достигнутого прогресса при дальнейших изменениях программы были добавлены тесты Minitest::Benchmark
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Следующие проблемы удалось найти и решить:

№1
Использование bang методов вместо обычных для сокращения аллоцированной памяти и числа объектов. Другие небольшие поправки в виде frozen objects, изменения существующих строк, а не создание новых, убрал парсинг дат и т.д.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Этому описанию не хватает конкретных цифр, значений метрики до и после оптимизации.
С цифрами было бы более информативно, и понятно, какой эффект имела та или иная оптимизация.
Группировка нескольких правок в один пункт ещё сильнее смешивает оптимизации и не даёт понять эффект каждой отдельной.

Использование bang методов вместо обычных для сокращения аллоцированной памяти и числа объектов. Другие небольшие поправки в виде frozen objects, изменения существующих строк, а не создание новых, убрал парсинг дат и т.д.

№2
Обновление версии Ruby до 2.6.1 для использования метода split без инициализации дополнительного массива в памяти при парсинге пользователей и сессий.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Насколько помогло обновление версии Ruby?

@@ -0,0 +1,29 @@
class Parser
USER_PREFIX = 'user,'.freeze
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ещё как вариант можно было бы использовать # frozen_string_literal: true

end

def bench_report
assert_performance_linear 0.99 do |file|
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants