More info regarding question #3: The performance problem is reproducable by creating comments on multiple different line numbers of the diff. Each line diff comment results in a seperate HTTPS GET request when the page is loaded. These are what the GET requests look like (in FireBug):
Firebug's log limit has been reached. %S entries not shown. Preferences GET https://reviewboard/r/3043/reviews/5911/fragment/diff-comment/9742/?1234569592 304 Not Modified 180ms jquery-1....1.min.js (line 19) GET https://reviewboard/r/3043/reviews/5961/fragment/diff-comment/9789/?1234569592 304 Not Modified 48ms jquery-1....1.min.js (line 19) GET https://reviewboard/r/3043/reviews/5964/fragment/diff-comment/9792/?1234569592 I think I have probably provided enough information to show the problem now. On Mar 9, 11:00 am, mary <ciaom...@gmail.com> wrote: > Results for question #3: > > I createed a reviewrequest with just one review with many comments and > it does not have the same performance issues. Additionally, in FireBug > the review is only showing one HTTPS GET request. In this test all the > comments and the review itself was made by just one user (myself.) > > I will try some additional tests to provide additional information > unless you have enough information, let me know. > > Thanks! > > On Mar 9, 10:11 am, mary <ciaom...@gmail.com> wrote: > > > > > Results for question #2: > > I installed FireBug and setup the console to show logging. I then > > brought up one of the reviews that has been a problem for us and > > showed hundreds of HTTPS GET requests for diff comments, each taking > > about ~200-300 milliseconds. It took ~6 minutes to fully load the > > page. > > > I then set the $("review.body").hide() and refreshed the page, and it > > still took almost 3 minutes to load. The same HTTPS GET requests were > > being displayed, but this time they only took ~90milliseconds and were > > showing "304 Not Modified". > > > This is my first use of FireBug so please advise if you'd like more or > > different information. > > > On Mar 6, 4:49 pm, Christian Hammond <chip...@chipx86.com> wrote: > > > > Thanks for the quick response. I'll be interested in seeing the results. > > > > I'm working on code now that should address these issues (though some > > > further decisions will be made based on #2 and #3). > > > > Essentially, we're looking at expanding/collapsing reviews based on the > > > following logic: > > > > For each review: > > > If the user has a pending reply to this review, expand it. > > > Else if the user has replied to the review, and there's no further > > > activity on the review, collapse it. > > > Else If the review is newer than the latest change description, AND > > > it's > > > the latest review from that user, expand it. > > > Else if there's activity on the review since the latest change > > > description and since the last time the user viewed the page, expand it. > > > Else, collapse it. > > > > Users can of course manually expand a review. > > > > This should keep the number of visible reviews quite low. Hopefully it'll > > > be > > > the set of reviews that the user actually wants to see. The review > > > contents > > > won't be loaded in unless the user does expand the review, so the page > > > should load a lot faster. > > > > This doesn't solve the issue of one single review with many hundreds of > > > comments, but I'm hoping that's not a common case, and that would have to > > > be > > > solved differently. > > > > The the above logic seems broken to someone, please let me know! > > > > Christian > > > > -- > > > Christian Hammond - chip...@chipx86.com > > > Review Board -http://www.review-board.org > > > VMware, Inc. -http://www.vmware.com > > > > On Fri, Mar 6, 2009 at 4:37 PM, mary <ciaom...@gmail.com> wrote: > > > > > answers inline below... > > > > > On Mar 6, 3:52 pm, Christian Hammond <chip...@chipx86.com> wrote: > > > > > A couple more questions. I'm playing around with a couple possible > > > > > fixes, > > > > > but need to find out more where the bottleneck is. Clearly it's > > > > > browser-side, but the question is whether it's the fact that there's a > > > > lot > > > > > in the DOM or whether the rendering is the slow part. > > > > > > 1) Is the page still slow once it fully loads? > > > > > Yes, the comment boxes that are used to write comments are slow even > > > > after the page is fully loaded. It doesn't make sense to me, but this > > > > is the behavior being seen by most everyone. (This is only the case > > > > for reviews with many comments.) > > > > > In addition to this, some folks are also complaining that the page > > > > cannot be used until the page is fully loaded - this has not been my > > > > experience but I wanted to pass it along too. > > > > > Also, we are seeing these issues on reviews that have 10-20 different > > > > file diffs. Now that I mention this I will try to repro with just one > > > > file diff to see if that makes a difference or not. > > > > > And finally, the CPU gets pegged when these pages are loading. > > > > > > 2) Can you install the Firebug extension for Firefox and, in the > > > > > console, > > > > > type the following: > > > > > > $(".review .body").hide() > > > > > I will get back to you on this question. > > > > > > And see if the page is now faster to interact with? (All the reviews > > > > > will > > > > be > > > > > hidden until you reload, so this is clearly not a fix by itself, but > > > > > will > > > > > tell us whether the bottleneck is the DOM or the rendering). > > > > > > 3) Are these comments spread across many reviews? Or does a single > > > > > review > > > > > usually have enough comments to cause problems by itself? > > > > > Yes, the comments are spread across many reviews. > > > > I will try to reproduce using a single review to provide more insight > > > > and get back to you. > > > > > > Christian > > > > > > -- > > > > > Christian Hammond - chip...@chipx86.com > > > > > Review Board -http://www.review-board.org > > > > > VMware, Inc. -http://www.vmware.com > > > > > > On Fri, Mar 6, 2009 at 3:10 PM, mary <ciaom...@gmail.com> wrote: > > > > > > > Thank-you for making this a priority! I'll keep an eye out for the > > > > > > fix > > > > > > and grab immediately. > > > > > > > Breaking up the reviews into smaller pieces is not a use case that > > > > > > is > > > > > > going down well here, but it is known. thanks again! > > > > > > > On Mar 6, 2:54 pm, Christian Hammond <chip...@chipx86.com> wrote: > > > > > > > We'd have to decide what we're doing to fix this first. Depending > > > > > > > on > > > > what > > > > > > > that is, it could take a few days to implement, or longer. We can > > > > make it > > > > > > a > > > > > > > priority for beta 1 (the next release), and of course you'd be > > > > > > > able > > > > to > > > > > > just > > > > > > > upgrade to a nightly once it's in. > > > > > > > > Short-term, I'd just advise splitting up the changes more, if > > > > possible. > > > > > > > Having smaller things to review should mean fewer comments. > > > > > > > > Christian > > > > > > > > -- > > > > > > > Christian Hammond - chip...@chipx86.com > > > > > > > Review Board -http://www.review-board.org > > > > > > > VMware, Inc. -http://www.vmware.com > > > > > > > > On Fri, Mar 6, 2009 at 2:51 PM, mary <ciaom...@gmail.com> wrote: > > > > > > > > > Yes, I mean comments not reviews. > > > > > > > > > I'm seeing the same issues on Alpha4 on my test server. It > > > > > > > > seems to > > > > me > > > > > > > > that the loading of the diff fragments across all the comments > > > > > > > > is > > > > > > > > causing our problems - loading such a review sometimes crashes > > > > > > > > the > > > > > > > > browser (i've seen this on firefox mostly) and in IE the page > > > > > > > > often > > > > > > > > shows a script error popup box part way through the load. > > > > > > > > > Changing the page size would help us so much, can you give any > > > > > > > > indication of a time frame for such a change? > > > > > > > > > We'd benefit from the other suggestions as well, but jsut > > > > > > > > getting > > > > > > > > something workable for medium-to-large reviews is our immediate > > > > > > > > concern. > > > > > > > > > Thanks! > > > > > > > > > On Mar 6, 2:39 pm, Christian Hammond <chip...@chipx86.com> > > > > > > > > wrote: > > > > > > > > > I think we should. The thing is that the newest review > > > > > > > > > request is > > > > at > > > > > > the > > > > > > > > > bottom, so it's kinda weird. > > > > > > > > > > Another thing we should look into is auto-collapsing old > > > > > > > > > reviews > > > > > > (such as > > > > > > > > > reviews made before the last update to the review request), > > > > allowing > > > > > > them > > > > > > > > to > > > > > > > > > expand again. This would fetch the collapsed items from the > > > > server > > > > > > > > > dynamically. > > > > > > > > > > Scalability of the review request page is certainly something > > > > > > > > > we > > > > > > should > > > > > > > > > tackle for 1.0. > > > > > > > > > > As far as using Alpha 2 vs. Alpha 4, if you use Alpha 4 the > > > > > > > > > page > > > > > > should > > > > > > > > load > > > > > > > > > pretty fast, with the diff fragments loading dynamically > > > > > > > > > after. > > > > Even > > > > > > with > > > > > > > > > 150+ comments (do you mean actual comments or reviews, btw?) > > > > > > > > > it > > > > > > shouldn't > > > > > > > > > take forever in alpha 4 to display those. Just might take a > > > > > > > > > while > > > > for > > > > > > > > those > > > > > > > > > diff fragments ot finish loading across all comments. > > > > > > > > > > Christian > > > > > > > > > > -- > > > > > > > > > Christian Hammond - chip...@chipx86.com > > > > > > > > > Review Board -http://www.review-board.org > > > > > > > > > VMware, Inc. -http://www.vmware.com > > > > > > > > > > On Fri, Mar 6, 2009 at 2:06 PM, David Trowbridge < > > > > trowb...@gmail.com > > > > > > > > > wrote: > > > > > > > > > > > Perhaps we need to paginate the reviews page in addition to > > > > > > > > > > the > > > > > > diff? > > > > > > > > > > > -David > > > > > > > > > > > On Fri, Mar 6, 2009 at 2:03 PM, mary <ciaom...@gmail.com> > > > > wrote: > > > > > > > > > > > > Thank you for your reply. The slowness is: > > > > > > > > > > > 1. page load takes several minutes (the more comments, the > > > > longer > > > > > > it > > > > > > > > > > > takes) > > > > > > > > > > > 2. typing a comment is very slow on reviews with many > > > > comments. > > > > > > > > > > > 3. Scrolling on the review page is painful when many > > > > > > > > > > > reviews > > > > > > > > > > > Developers are speculating its due to a huge DOM and say > > > > > > > > > > > that > > > > > > > > > > > performance benchmarks seem relative to the document size, > > > > > > > > complexity, > > > > > > > > > > > and browser type (Safari works best, then FireFox, then > > > > > > > > > > > IE.) > > > > > > > > > > > > Can we change the paging size the ReviewBoard uses? That > > > > would > > > > > > help > > > > > > > > us > > > > > > > > > > > most likely. > > > > > > > > > > > > Further details: > > > > > > > > > > > yes, we're using memcache. 4G ram. > > > > > > > > > > > > We've been running Alpha2 the past couple weeks. But it > > > > > > > > > > > seems > > ... > > read more »- Hide quoted text - > > - Show quoted text - --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "reviewboard" group. To post to this group, send email to reviewboard@googlegroups.com To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -~----------~----~----~----~------~----~------~--~---
