Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimized version of task-2 #123

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bshourse
Copy link

Поревьюйте пожалуйста.

Описание в case-study.md

Copy link
Collaborator

@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.

Time taken: 7.52 seconds
Memory usage: 17 MB

В бюджет уложился. Время обработки снизилось в ~3.26 раз от значений полученых после оптимизации в задании 1.
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 👍


Вот тут начались трудности, я пытался не делать это через сплит, парсил посимвольно, даже написал что-то.
Сначала я добавил построчное чтение с foreach, результат особо не изменился видимо потому что файл слишком маленького размера и проблема скорее не в загрузки фала в память а в агригировании данных в памяти создания дополнительных объектов. Тут я пока не понял как они по какой формуле можно посчитать заранее какое количество объектов того или иного класса будет на большем объеме данных(нужно заново смотреть ваши ведео)
Потратил очень много времени, минус моральнулся потому, что от сплита избаивался кое как, а осталось понимание что есть куча мест с агригированием данных которые делаются в памяти(я это помнил но уделил внимание на точку росту которая показывалась memory profile-ром.
Copy link
Collaborator

Choose a reason for hiding this comment

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

да, тут разные вещи смешались и вас запутали

  1. количество аллокаций

  2. максимальный объём RSS

может быть сколько угодно много аллокаций, но если это временные объекты и GC может их удалять, то MAX RSS расти не будет

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

но потренироваться находить и убирать лишние аллокации тоже полезно; если объект не создавать, то и не придётся его потом удалять - profit


def profile
profile_memory do
profile_time do
Copy link
Collaborator

Choose a reason for hiding this comment

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

не надо смешивать много всего сразу

профилирование памяти отдельно - замеры времени отедльно - замеры памяти отдельно

наличие профилировщика тормозит работу и раздуавает память, поэтому мерить надо всё без профилировщика

describe '#performance check' do
context 'when file contains 3_250_940 lines' do
it 'performs under 10 seconds' do
expect { service.call(input_file_name: 'data_large.txt') }.to perform_under(10).sec
Copy link
Collaborator

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