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

Homework solution #150

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

Conversation

araslanov-e
Copy link

  • Оптимизировал программу для обработки больших данных менее 30 секунд
  • Добавил тест производительности
  • Описал этапы оптимизации


def test_result
work(file_name: 'data.txt')
expected_result = '{"totalUsers":4,"uniqueBrowsersCount":14,"totalSessions":15,"allBrowsers":"CHROME 13,CHROME 20,CHROME 35,CHROME 6,FIREFOX 12,FIREFOX 32,FIREFOX 47,INTERNET EXPLORER 10,INTERNET EXPLORER 28,INTERNET EXPLORER 35,SAFARI 17,SAFARI 29,SAFARI 39,SAFARI 49","usersStats":{"Leida Cira":{"sessionsCount":6,"totalTime":"455 min.","longestSession":"118 min.","browsers":"FIREFOX 12, INTERNET EXPLORER 28, INTERNET EXPLORER 28, INTERNET EXPLORER 35, SAFARI 29, SAFARI 39","usedIE":true,"alwaysUsedChrome":false,"dates":["2017-09-27","2017-03-28","2017-02-27","2016-10-23","2016-09-15","2016-09-01"]},"Palmer Katrina":{"sessionsCount":5,"totalTime":"218 min.","longestSession":"116 min.","browsers":"CHROME 13, CHROME 6, FIREFOX 32, INTERNET EXPLORER 10, SAFARI 17","usedIE":true,"alwaysUsedChrome":false,"dates":["2017-04-29","2016-12-28","2016-12-20","2016-11-11","2016-10-21"]},"Gregory Santos":{"sessionsCount":4,"totalTime":"192 min.","longestSession":"85 min.","browsers":"CHROME 20, CHROME 35, FIREFOX 47, SAFARI 49","usedIE":false,"alwaysUsedChrome":false,"dates":["2018-09-21","2018-02-02","2017-05-22","2016-11-25"]},"Evgeny Araslanov":{"sessionsCount":0,"totalTime":"0 min.","longestSession":" min.","browsers":"","usedIE":false,"alwaysUsedChrome":true,"dates":[]}}}' + "\n"
Copy link
Author

Choose a reason for hiding this comment

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

Вспомнил момент, что тут баг есть, у "Evgeny Araslanov" нет сессий, но при этом в результате "alwaysUsedChrome":true

config.include RSpec::Benchmark::Matchers
end

describe 'Performance' do
Copy link
Author

Choose a reason for hiding this comment

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

И тут ещё понял, что наверное стоит добавить в тест проверку кол-ва строк в файле data_large.txt, чтобы мы были уверены на каком количестве данных ожидаем такой результат

Copy link
Collaborator

Choose a reason for hiding this comment

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

ну в целом хорошо бы чтобы число строк было наглядно, да

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.

Nice work!

Программа поставлялась с тестом. Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации.

## Feedback-Loop
Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за 10-20 секунд.
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

## Feedback-Loop
Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за 10-20 секунд.

Вот как я построил `feedback_loop`: Создавал файл с N строк, чтобы программа могла выполнятся 10-20 секунд
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

### Ваша находка №1
- CallStack показал что программа 94.55% времени тратит на Array#select. С помощью Graph стало понятно что много времени тратиться на поиск сессий пользователя.
- Для решения данной проблемы, перед тем как проходится по пользователям, нужно подготовить данные users_sessions в виде хэша, ключем будет id пользователя, а значение массив его сессий, тогда мы один раз пройдемся по sessions
- Время выполнения программы уменьшилось с ~20 секунд до ~1 секунды, при входных данных в 30000 строк. Время выполнения программы на большом объёме данных больше 30 секунд.
Copy link
Collaborator

Choose a reason for hiding this comment

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

тут самое главное, что асимптотику сильно лучше сделали


### Ваша находка №4
- При объёме данных в 400к строк CallStack показал что программа 52.94% времени тратит на Array#all?. С помощью Graph стало понятно что много времени тратиться на проверку уникальности браузера (uniqueBrowsers).
- Для этих данных лучше всего подойдет Set.
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

config.include RSpec::Benchmark::Matchers
end

describe 'Performance' do
Copy link
Collaborator

Choose a reason for hiding this comment

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

ну в целом хорошо бы чтобы число строк было наглядно, да


report['usersStats'] = {}

# Собираем количество сессий по пользователям
collect_stats_from_users(report, users_objects) do |user|
{ 'sessionsCount' => user.sessions.count }
end
progressbar.increment
Copy link
Collaborator

Choose a reason for hiding this comment

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

progressbar сам по себе может довольно сильно тормозить, лучше делать его отключаемым / опциональным


# Собираем количество времени по пользователям
collect_stats_from_users(report, users_objects) do |user|
{ 'totalTime' => user.sessions.map {|s| s['time']}.map {|t| t.to_i}.sum.to_s + ' min.' }
end
progressbar.increment
Copy link
Collaborator

Choose a reason for hiding this comment

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

а, тут progressbar не по кол-ву юзеров/строк, а по кол-ву этапов программы

можно и так для красоты, либо просто путсами

Copy link
Author

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