github.com/shurcooL/issuesapp/...

issues/kv: Poor performance when large number of issues. github.com/shurcooL/issuesapp#29

Closeddmitshur opened this issue 8 years ago
dmitshur commented 8 years ago

When there are many open and/or closed threads, the overview page can take excessively long time to render:

Image

This thread tracks that issue. I've done some investigation and I can see the source of poor performance.

Write Preview Markdown
dmitshur commented 8 years ago

Adding some notes for future reference.

Rendering the index page for tracker invokes 3 service methods: List, Count(open), Count(closed).

Here are 3 timings for repo with 2~ issues (total time = 300-400 ms):

Count(open) taken: 1.381118ms
Count(closed) taken: 1.38761ms
List taken: 4.236063ms

Count(open) taken: 1.358867ms
Count(closed) taken: 1.297077ms
List taken: 1.696988ms

Count(open) taken: 1.289523ms
Count(closed) taken: 1.250312ms
List taken: 1.608113ms

Here's a repo with 183 open + 61 closed issues (total 800-900 ms):

Count(open) taken: 92.806825ms
Count(closed) taken: 98.225802ms
List taken: 243.129079ms

Count(open) taken: 111.360533ms
Count(closed) taken: 102.080099ms
List taken: 192.125316ms

Count(open) taken: 93.992722ms
Count(closed) taken: 101.173754ms
List taken: 209.102994ms

The current Count implementation has no index so it has to scan ​_each_​ issue (that's opening/reading/parsing 183+71 JSON files) on each render... and do that twice (once for open, another time for closed count).

The List operation also needs to get the number of comments for each issue it returns (e.g., 183 open issues). That means it needs to read contents of 183 buckets and count the number of entries in each.

I think we'll need to add an index for List/Count operations to speed them up, otherwise tracker will get slow when many threads are added, and pagination won't help. There's already some indexing code for search, this can go side-by-side with that. /cc @renfredxh

Write Preview Markdown
dmitshur commented 8 years ago

Another opportunity to optimize. There are a few cases where the templates call out a method more than once, which needlessly performs the computation multiple times.

For example, this template:

{{define "issue"}}
	<h1>{{.Issue.Title}} <span class="gray">#{{.Issue.ID}}</span></h1>
	<div id="issue-state-badge" style="margin-bottom: 20px;">{{template "issue-state-badge" .Issue}}</div>
	{{if .Issue.Reference}}{{template "reference" .Issue}}{{end}}
	{{range .Items}}
		{{template "issue-item" .}}
	{{end}}
	<div id="new-item-marker"></div>
	{{template "new-comment" .}}
{{end}}

Should become:

{{define "issue"}}
	{{$issue := .Issue}}
	<h1>{{$issue.Title}} <span class="gray">#{{$issue.ID}}</span></h1>
	<div id="issue-state-badge" style="margin-bottom: 20px;">{{template "issue-state-badge" $issue}}</div>
	{{if $issue.Reference}}{{template "reference" $issue}}{{end}}
	{{range .Items}}
		{{template "issue-item" .}}
	{{end}}
	<div id="new-item-marker"></div>
	{{template "new-comment" .}}
{{end}}

To reduce number of calls to Issues() method from 5 times to just 1 time.

Write Preview Markdown
dmitshur commented 7 years ago

I believe the viable template optimizations, like the one mentioned in comment above, have been done by now.

Write Preview Markdown
dmitshur commented 7 years ago

I think that all low-hanging fruit optimizations on issuesapp side have been done (see this comment), and performance is quite good now.

issues/kv doesn't exist anymore, but issues/fs is very similar. Still, that is located in another repository and any performance issues can be tracked there. So, closing this issue here.

Write Preview Markdown
dmitshur closed this 7 years ago
to comment.