-
Notifications
You must be signed in to change notification settings - Fork 193
Homework 0.0.1 #1
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
Conversation
spajic
left a comment
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.
Оставил несколько комментов к коду.
В целом тут есть ещё над чем поработать, чтобы уложиться в бюджет.
Попробуйте разные профилировщики, более точно разобраться куда уходит время за счёт разбиения жирных методов на подметоды.
case-study.md
Outdated
| @@ -0,0 +1,15 @@ | |||
| 1. выключил ГК поставил руби проф | |||
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.
Case-study хотелось бы более подробно и ближе к шаблону.
Самое главное - хотелось бы по шагам видеть главную точку роста в профилировщике, как она была исправлена и какой был эффект.
case-study.md
Outdated
| @@ -0,0 +1,15 @@ | |||
| 1. выключил ГК поставил руби проф | |||
| 2. Date#parse 22.09 -> 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.
Подсказка, надо подумать, зачем вообще делается парсинг даты.
task-1.rb
Outdated
| totalTime: user.sessions.map { |s| s[:time].to_i }.sum.to_s + ' min.', | ||
| longestSession: user.sessions.map { |s| s[:time].to_i }.max.to_s + ' min.', | ||
| browsers: user.sessions.map { |s| s[:browser] }.sort.join(', '), | ||
| usedIE: user.sessions.any? { |s| s[:browser] =~ /INTERNET EXPLORER/ }, |
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.
Регэкспы медленные
spajic
left a comment
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.
approve + some comments
|
|
||
|
|
||
| ## Формирование метрики | ||
| Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: Wall Time |
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.
Wall Time чего?
| Программа поставлялась с тестом. Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. | ||
|
|
||
| ## Feedback-Loop | ||
| Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за 1s-2s |
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.
1-2 секунды - то что надо 👍
|
|
||
| Вот какие проблемы удалось найти и решить | ||
|
|
||
| - Многочисленое использование бесполезных map |
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.
А это была главная точка роста?
Каким отчётом нашли?
Как поменяли и какой был эффект?
Хотелось бы видеть эту информацию в case-study.
| RSpec.describe "File Parsing" do | ||
| context "check performance" do | ||
| it "should take us less than 30 seconds" do | ||
| expect { work("data_large.txt", disable_gc: true) }.to perform_under(30).sec |
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.