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

[Yaroslavzev] Homework #140

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

Yaroslavzev
Copy link

В этом PR выполненное задание к первой неделе курса по оптимизации.

В результате проделанной оптимизации наконец удалось обработать файл с данными в заданный бюджет 30 секунд.
Измеренный прирост производительности составил 33 раз для обработки файла в 10 000 строк

Заданный бюджет выполнения программы в 30 секунд был достигнут на M1


Вот как я построил `feedback_loop`: *как вы построили feedback_loop*
Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за _время, которое у вас получилось_
Copy link
Collaborator

Choose a reason for hiding this comment

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

_it was a placeholder_


- флейм граф показал, что Array#select занимает значительное время исполнения
- Array#select происводит поиск по всему массиву сессий и это занимает много времени. Для ускорения поиска, я изменил структуру оранизации сессий и в реализации V1 сессии сгруппированы по user_id в хеш. Таким образом, используются оптимизированный механизмы поиска ruby и достигается прирост происховодительности
- Скрость выполнения увеличина в 4 раза для 4000 строк
Copy link
Collaborator

Choose a reason for hiding this comment

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

тут главное, что асимптотика стала качественно лучше и приблизилась к линейное

Потому что фетч из хэша это O(1), а select из массива это O(N)

- как изменился отчёт профилировщика - исправленная проблема перестала быть главной точкой роста?
- флейм граф показал, что Array#+ занимает значительное время исполнения
- На основе https://github.com/fastruby/fast-ruby?tab=readme-ov-file#arrayconcat-vs-array-code Array#+ можно заменить на Array#concat
- Замена Array#+ на Array#concat дала прирост как минимум 10% на 4000 строк
Copy link
Collaborator

Choose a reason for hiding this comment

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

4000 строк это маловато уже

хорошая идея взять какую-то часть входных данных, чтобы программа не выполнялась вечность, но хоршо подбирать объём данных так, чтобы программа успевала покрутиться пару секунд. Если она завершается слишком быстро (“не успевает поработать”) могут возникнуть какие-то перекосы (например, на полном объёме основная часть времени тратится в основном цикле, а если данных мало - то большая часть уходит на инициализацию и финализацию, например на чтение из файла и запись потом в файл)

И плюс когда время уже на миллисекунды - больше влияние погрешностей.


### Ваша находка №3

- флейм граф показал, что метод String#split и Date#parse занимает значительное время
Copy link
Collaborator

Choose a reason for hiding this comment

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

всегда лучше по одному делать, иначе не понятно что из двух изменений как сыграло

@@ -0,0 +1,16 @@
source 'https://rubygems.org'

ruby '3.2.2'
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

confidence: 95
)

x.report('slow string concatenation') do
Copy link
Collaborator

Choose a reason for hiding this comment

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

wrong name

end

it 'works faster' do
expect { WorkV5.work(file_name) }.to perform_faster_than { WorkV4.work(file_name) }.at_least(1.05).times
Copy link
Collaborator

Choose a reason for hiding this comment

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

perform_faster_than - nice 👍


it 'performs perform_power' do
expect { |_n, i| InitWork.work(file_names[i]) }.to perform_power.in_range(8, 32_768).ratio(8)
expect { |_n, i| WorkV5.work(file_names[i]) }.to perform_linear
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

file_lines.each do |line|
cols = line.split(',')
users += [parse_user(line)] if cols[0] == 'user'
sessions += [parse_session(line)] if cols[0] == 'session'
Copy link
Collaborator

Choose a reason for hiding this comment

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

вроде же в case-study пришли к тому, что так плохо массив наполнять, надо <<

uniqueBrowsers = []
sessions.each do |session|
browser = session['browser']
uniqueBrowsers += [browser] if uniqueBrowsers.all? { |b| b != browser }
Copy link
Collaborator

Choose a reason for hiding this comment

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

use Set / SortedSet

@@ -0,0 +1,67 @@
# frozen_string_literal: true

module V5
Copy link
Collaborator

Choose a reason for hiding this comment

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

А, я понял вашу концепцию, что есть 5 версий кода. Возможно тогда какие-то комменты выше не актуальны, но я уже запутался

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.

Возможно я что-то лишнее накомментил из-за пяти версий, в целом всё гуд 👍

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