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

Add JMH test #123

Merged
merged 9 commits into from
Mar 5, 2019
Merged

Add JMH test #123

merged 9 commits into from
Mar 5, 2019

Conversation

viartemev
Copy link
Owner

@viartemev viartemev commented Feb 25, 2019

issues: #88, #115

@codecov
Copy link

codecov bot commented Feb 25, 2019

Codecov Report

Merging #123 into master will increase coverage by 0.39%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #123      +/-   ##
============================================
+ Coverage     37.64%   38.03%   +0.39%     
  Complexity       43       43              
============================================
  Files            20       20              
  Lines           255      255              
  Branches         43       43              
============================================
+ Hits             96       97       +1     
+ Misses          143      142       -1     
  Partials         16       16
Impacted Files Coverage Δ Complexity Δ
...n/com/viartemev/thewhiterabbit/channel/Channels.kt 38.09% <0%> (+2.38%) 3% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4b3c62...b91e495. Read the comment docs.

@viartemev viartemev force-pushed the add-jmh branch 2 times, most recently from 0af4fd5 to 3d0f998 Compare February 25, 2019 10:52
@viartemev viartemev changed the title Add JHM test Add JMH test Feb 25, 2019
@asm0dey
Copy link
Collaborator

asm0dey commented Feb 27, 2019

У меня несколько комментариев, которые не относятся напрямую к коду:

  1. Надо бы JMH вынести в отдельный подпроект, в грэдле так можно, чтобы не применять плагин на всё приложение и не светить зависимостями на всё приложение
  2. Надо как минимум для сравнения добавить публикацию без подтверждения
  3. бенчмаркать, кажется, надо одиночные отправки сообщения, но я не знаю можно ли это сделать
  4. Надо бы добавить обычную публикацию сообщений через стандартный клиент кролика
  5. Надо бы добавить 10_000 сообщений тоже

@viartemev
Copy link
Owner Author

viartemev commented Feb 27, 2019

@asm0dey

  1. Профиты какие?
  2. 👍
  3. @Param("1", "10", "100", "1000") - бенчмаркается 1, 10, 100 и 1000 сообщений, почему именно 1?
  4. А зачем обычная отправка сообщений из кролика?
  5. @Param("1", "10", "100", "1000") - параметр, сколько сообщений отправлять

@asm0dey
Copy link
Collaborator

asm0dey commented Feb 27, 2019

  1. Так я прям в первом пункте написал — зачем тебе во врем яобычного билда поиск аннотаций всяких, кодогенерация и вот это всё?
  2. потому что то что на самом деле важно — это сколько по времени отправляется ОДНО сообщение. А разные количества нужны чтобы посмотреть на разную степень прогретости
  3. Потому что это вменяемый бейзлайн — если мы медленнее — то надо разбираться почему
  4. ага, вот туда надо добавить 10_000, почему — см п. 3

@viartemev viartemev force-pushed the add-jmh branch 4 times, most recently from ccf1aa6 to 25946fb Compare March 5, 2019 10:35
@viartemev viartemev merged commit 6a56576 into master Mar 5, 2019
@delete-merged-branch delete-merged-branch bot deleted the add-jmh branch March 5, 2019 14:49
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