783973 - Enhance performance by only partially loading comments by default

Status: NEW

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Jean-Marc Desperrier]

The recent discussion comparing github against bugzilla shows that one of the most noticed bad point of bugzilla is the low perceived performance, confirmed by roc.

My intuitive guess is that the loading of comments represents a significant part of the whole load on bugzilla.
Therefore, a low hanging fruit for getting significant improvements would be to stop systematically loading all comments.

By default only the most recent comments would be loaded, say the 10 or 5 last ones, and a div that's put in front of the list of comment would allow to request the load the complete list when needed. Ideally, users would be able to change the default number of displayed comments in their profile.

The following French site implemented just that as a quick hack solution to a load problem and it proved so effective that they kept it definitively afterward ( see http://www.universfreebox.com/article18047.html#bas_corp, "Afficher les xxx premiers commentaires" means "Show the xxx first comments". They are dynamically loaded when you click on the sentence). This specific implementation demonstrates this feature is not too difficult to integrate inside a page that is not very ajax-ified on the whole.

Comment 1

[Frédéric Buclin]

The problem is that if JS is disabled, you wouldn't get the other comments at all. I would have to do some benchmarking, but I already know that collecting data from the DB is very fast. What is slow is to pass them to your browser. Bugzilla 4.2 and 4.4 have better performance compared to 4.0. I don't know if perf improvements have been backported to bmo 4.0.

Comment 2

[Gervase Markham [:gerv]]

I think we should measure first, optimize second. See bug 783545.

Gerv

Comment 3

[Justin Lebar (not reading bugmail)]

If you do this, please provide a pref to let me load all the comments.  I would /really/ not like this, and I can't imagine I'm alone!

Comment 4

[David Lawrence [:dkl]]

(In reply to Frédéric Buclin from comment #1)
> The problem is that if JS is disabled, you wouldn't get the other comments
> at all. I would have to do some benchmarking, but I already know that
> collecting data from the DB is very fast. What is slow is to pass them to
> your browser. Bugzilla 4.2 and 4.4 have better performance compared to 4.0.
> I don't know if perf improvements have been backported to bmo 4.0.

One idea if we had a user preference such as 

Number of comments to preload (requires javascript):
 - All (default)
 - 10 
 - 20
 - 30

And then $bug->comments would look at this preference setting when loading the comments from the longdescs table. This would down and the amount of data passed to the templates.

We can then update Bug.comments to take the offset from the user preference and then use an ajax call to load the remaining comments upon demand.

As gerv mentioned, we need to see if this would even give that much of improvement. For very large bugs I suspect it would, but probably not much for bugs with only 10-30 comments.

BMO (sanitized) has around 150 bugs with more than 300 comments each (5 with over 900). But those are very few considering the total number of bugs. The average amount of comments for all bugs is 8.4793 so I am not sure even preloading 10 would be helpful in most cases.

Another option glob and I have discussed in the past is to use a memcached daemon on BMO to cache the compiled templates for specific comments in memory. Comments change very rarely if at all so would be great for that. As well as user templates embedded all over in Bugzilla.
Anything that doesn't change very often.   

dkl

Comment 5

[Jean-Marc Desperrier]

(In reply to Frédéric Buclin from comment #1)
> The problem is that if JS is disabled, you wouldn't get the other 
> comments at all. [...] 

You could put a hard link on the div with a "show all comments" parameter.

> collecting data from the DB is very fast.
> What is slow is to pass them to your browser.

If so there'd still be a gain from passing less to the browser, but only when you actually show a lot of comment, and if the average number of comments displayed is in fact low ...

This said I remember analyzing a DB problem once where the problem actually was not the speed of each request, they were rather blazingly fast, almost all in the low ms range, but in the fact that getting one application level result involved running several thousands DB requests.

(In reply to Gervase Markham [:gerv] from comment #2)
> I think we should measure first, optimize second. See bug 783545.

I'd rather measure the time as seen from the application level, than the speed of each request.

It seems there's a number of reason to doubt partially loading comments would really enhance things, but there's certainly a number of things that do slow thing down.
I'll open a discussion on dev.apps.bugzilla about it.

Comment 6

[Justin Lebar (not reading bugmail)]

This is probably not useful (because drive-by performance suggestions are not usually useful), but on the off chance it is: Surely we could run some of those DB queries in parallel?  Might that speed things up?

Comment 7

[David Lawrence [:dkl]]

(In reply to Justin Lebar [:jlebar] from comment #6)
> This is probably not useful (because drive-by performance suggestions are
> not usually useful), but on the off chance it is: Surely we could run some
> of those DB queries in parallel?  Might that speed things up?

There is some work currently going with regard to DB query optimization:
https://bugzilla.mozilla.org/show_bug.cgi?id=783545

dkl

Comment 8

[Frédéric Buclin]

Some data:

Loading a bug with 180 comments in it shows that 0.4 second is spent to format the comment creation date, and another 0.25 second is spent to linkify stuff in comments. This means 0.65 second for comments themselves.

Comment 9

[Gervase Markham [:gerv]]

LpSolit: Out of a total of how long?

Why do you think formatting comment creation dates takes so long? Can we write a special-purpose function to use for this instead of whatever general library we are using? Or does that time include loading some heavyweight modules we can avoid loading?

Gerv

Comment 10

[:glob ✱]

bug 829413 is relevant here -- currently we always load all comments, even if a subset is requested (although there may be no tangible cost for doing this, investigations are ongoing).

Comment 11

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #9)
> LpSolit: Out of a total of how long?

I don't remember by heart: out of a few seconds. The important point is that more than half a second is spent to format comment dates.


> Why do you think formatting comment creation dates takes so long?

The reason is clearly to get the local timezone, see bug 829601. 0.2 second is spent there to get it. Caching it as suggested in bug 829601 would save us a significant part of the time spent in comments, especially with fewer comments: instead of having (x+0.2) second spent there, you would need x second only. For bugs with few comments, we know that x << 0.2 second.