-
Notifications
You must be signed in to change notification settings - Fork 181
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
Task 1 Denis Pavlov #124
base: master
Are you sure you want to change the base?
Task 1 Denis Pavlov #124
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В целом ок, хорошо бы было заполнить плейсхолдеры и написать за сколько получилось уложиться в результате.
Но понравился минимализм, всего лишь два изменённых файла. Некоторые коммитят по 150к строк
Я решил исправить эту проблему, оптимизировав эту программу. | ||
|
||
## Формирование метрики | ||
Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: *тут ваша метрика* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*тут был плейсхолдер*
Программа поставлялась с тестом. Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. | ||
|
||
## Feedback-Loop | ||
Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за *время, которое у вас получилось* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*тут тоже*
- какой отчёт показал главную точку роста | ||
По отчету Stackprof увидел большие затраты на Date.parse при сборе дат сессий пользователей, 34 процента общего времени | ||
- как вы решили её оптимизировать | ||
Использовать явный формат Date.strptime так как даты в файле в одном формате |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Дата на самом деле сразу в нужном формате, можно с ней ничего не делать
|
||
*Какими ещё результами можете поделиться* | ||
Было открытием для меня что один из способов собрать обработать уникальные данные - пройтись по массиву и убрать эти данные в множество Set, | ||
которое априори уникально, что это очень медленно. Это типичный паттерн в пайтоне насколько знаю. Оказывается собрать уникальные браузеры через хэщ быстрее на порядок. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Очень странно, насколько я знаю Set внутри работает примерно как и Hash. И многие ребята используют Set в этом задании. Бывает пишут, что чуть медленнее Hash, но не на порядок, а чуть-чуть
if br_hash[browser] | ||
else | ||
uniqueBrowsers << browser | ||
br_hash[browser] = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
двойная работа осталась
|
||
{ | ||
'sessionsCount' => user.sessions.count, | ||
'totalTime' => "#{sessions_timings.inject(:+)} min.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inject(:+) -> sum
'alwaysUsedChrome' => !user_browsers.any? { |b| b !~ /CHROME/ }, | ||
'dates' => user.sessions.map { |s| s['date'] } | ||
.map { |d| Date.strptime(d, '%Y-%m-%d') } | ||
.sort { |a, b| b <=> a } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverse
'dates' => user.sessions.map { |s| s['date'] } | ||
.map { |d| Date.strptime(d, '%Y-%m-%d') } | ||
.sort { |a, b| b <=> a } | ||
.map(&:iso8601) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не надо
No description provided.