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

Fix performance #44

Merged
merged 4 commits into from
Mar 26, 2016
Merged

Fix performance #44

merged 4 commits into from
Mar 26, 2016

Conversation

masutaka
Copy link
Owner

Before

$ time bundle exec ruby bin/github-nippou -s 20160325
(snip)
bundle exec ruby bin/github-nippou -s 20160325  1.50s user 0.27s system 10% cpu 16.724 total

After

$ time bundle exec ruby bin/github-nippou -s 20160325
(snip)
bundle exec ruby bin/github-nippou -s 20160325  1.49s user 0.28s system 16% cpu 10.603 total

last_response = @client.last_response
$t_after = Time.now.instance_eval { self.to_i * 1000 + (usec/1000) }
puts "ccc: #{$t_after - $t_before}";
$t_before = $t_after

while last_response.rels[:next] && in_range?(user_events.last)
Copy link
Owner Author

Choose a reason for hiding this comment

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

やはり GitHub へのアクセス回数がパフォーマンスダウンの影響。

ここの loop に入ると 30 行目の last_response = last_response.rels[:next].get でアクセスしてしまう。

そうならないように、17 行目の user_events = @client.user_events(@user, per_page: 100) のように Pagination を調整すれば良さそう。

Copy link
Owner Author

Choose a reason for hiding this comment

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

10〜100 に変更してパフォーマンスを図ったところ、雑に増やしても影響はないことが分かった。Ruby の処理が増えるだけなので。

 10: 13.860 sec
 20: 12.427 sec
 30: 12.546 sec (default)
 40: 10.451 sec #<= 今回はここから1ページに収まった
 50: 11.125 sec
 60: 11.699 sec
 70: 11.467 sec
 80: 11.329 sec
 90: 11.125 sec
100: 11.463 sec
200: 11.317 sec

雑に 100 にしよう。

@@ -106,7 +106,7 @@ def hash_for_issue(repo_name, issue_number)
title: issue.title.markdown_escape,
repo_basename: repo_name,
username: issue.user.login,
merged: client.pull_merged?(repo_name, issue.number),
merged: issue.merged,
Copy link
Owner Author

Choose a reason for hiding this comment

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

#44 (comment) に書いた、GitHub へのアクセス回数を減らす施策。

このときの issue はすぐ上にある client.issue(repo_name, issue_number) で取得した変数。現在の Issue を指し示しているため、Octokit::Client#pull_merged? で GitHub にアクセスする必要はない。

@masutaka masutaka merged commit 8631175 into master Mar 26, 2016
@masutaka masutaka deleted the fix-performance branch March 26, 2016 11:12
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.

None yet

1 participant