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-1 #155

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

Conversation

bshourse
Copy link

@bshourse bshourse commented Jan 28, 2025

Добавил пару папок:

  • benchmark - в ней лежат замеры производительности а также md файлы с описанием кейсов, которые находил

  • profiling - скрипт профайлера и json-ы для speedscope

  • spec - тесты

  • добавил несколько файлов с data на которых проводил работы.

~ 25 секунд на полном объеме данных

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

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! ✅

Далее внутри цикла использовать вытаскивать сессии для конкретного пользователя по хешу (O(1))

**Результат после оптимизационных действий**
На 25 тысяч строк метрика снизилась c 5.8 секунд до 632 ms (~ в 9.1 раза)
Copy link
Collaborator

Choose a reason for hiding this comment

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

да, и главное сложность линейная стала

=begin
puts 'Starting benchmark...'

Benchmark.ips do |x|
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

у нас тут скорее много секунд на итерацию, поэтому проще просто в секундах время смотреть


на

`user.sessions.map { |s| s['date'] }.sort.reverse` -- можно не вызывать 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.

с датой можно ничего не делать, да


`user.sessions.map { |s| s['date'] }.sort.reverse` -- можно не вызывать 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.

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

```

**Решение**
Использую Set для выбора уникальных браузеров
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,46 @@
## Case 3
Copy link
Collaborator

Choose a reason for hiding this comment

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

по отдельным файлам сложновато читать, так как они отображаются вразнобой

как вариант можно их как то назвать вроде case_1_..., case_2_...

### Пишу спеку (для фиксации текущей метрики)
`rspec spec/work_performance_spec.rb:25`

### Применил профилировщик stackprof speedscope
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 stackprof + speedscope = one love

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