438716 - Series data does not load

Status: VERIFIED FIXED

(Webtools Graveyard :: Graph Server, defect)

Description

[alice nodelman [:alice] [:anode]]

When opening the main page of the series data for the new front end it is not uncommon to click through two or three unresponsive script warning dialogs.

We are probably pre-loading too much data, or blocking on having all data available.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Info from the duplicate bug:

STEPS TO REPRODUCE:
1)  Disable the slow script dialog.
2)  Load http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox
3)  Click the "tp_l_1" link on a cycle of "WINNT 5.1 talos trunk qm-pxp01"
4)  Wait a few minutes (I waited 3 minutes or so) during which the browser
    consumes 100% CPU and is unresponsive.
5)  Start profiler
6)  Profile for a bit
7)  Stop profiler
8)  Kill the unresponsive browser

ADDITIONAL INFORMATION: All the time is spent under the load completion of an
XMLHttpRequest, executing JS.  About 1/8 of the total time is spent in security
checks during said JS execution, and a small amount spent on changing
attributes on <options>, but most of the time is JS execution itself.

This makes it impossible to get per-page numbers that would be needed in
debugging a Tp regression.

Comment 3

[Ryan Doherty (:rdoherty)]

(In reply to comment #0)
> We are probably pre-loading too much data, or blocking on having all data
> available.
> 

Yeah, after some network analysis, it's loading many megabytes of data. We should decrease the granularity so we only need a subset of the data or split it into multiple chunks.

Comment 4

[alice nodelman [:alice] [:anode]]

We don't get quite the same poor behavior when using the new graph server front end - which will become the default front end soon.  There are still perf issues to be handled (ie, you still end up with slow script warnings) but it doesn't crash.

You can play around with it at graphs.mozilla.org/g2/graph.html and graphs.mozilla.org/g2/graph.html#type=series

Since we are moving to the new front end we will not be fixing this issue in the old code.  

Comment 5

[Ryan Doherty (:rdoherty)]

I've done some initial performance tests/improvements. Using GranParadiso yields huge performance improvements. If you can't wait, use it.

If I reduce the timespan for the charts, I can speed things up too. Can we reduce it to 15 days? This could be a temporary solution until we do some major fixes.

I was also able to improve performance of one function that was called over 3k times.

Comment 6

[Ryan Doherty (:rdoherty)]

Er, by GranParadiso, I meant Minefield.

Comment 7

[Ryan Doherty (:rdoherty)]

Attachment: Performance improvements [checked in]

Here's some quick performance improvements. Changed regexes to indexOfs and removed some data from getdata.cgi calls that didn't seem to be in use (but still might be).

Comment 8

[Ryan Doherty (:rdoherty)]

Our biggest performance problem now is when jQuery evals the returned json. It's about 5 seconds with my test data. We have to reduce the amount of data returned from getdata.cgi. 

I'll start looking into this.

Comment 9

[alice nodelman [:alice] [:anode]]

Comment on attachment 329760 [details] [diff] [review]
Performance improvements [checked in]

This looks reasonable to me, and I'm all for faster page load times.

Comment 10

[Ryan Doherty (:rdoherty)]

Committed changes in changeset 71d9f8c152d0 (http://hg.mozilla.org/index.cgi/graphs/rev/71d9f8c152d0)

Comment 11

[Ryan Doherty (:rdoherty)]

Quick update before I'm gone. Some perf improvements were made, we won't know the real benefits until they are on production. Next steps would require a substantial re-write of some of the backend and frontend.

Comment 12

[Ryan Doherty (:rdoherty)]

I'm trying to reproduce from comment #2 and can't find the link specified on http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox

Can I get a direct url to test with?

Comment 13

[Ryan Doherty (:rdoherty)]

Changing title to a more correct one. Looks like a query is taking too long: bug 450640

Comment 14

[Ryan Doherty (:rdoherty)]

Attachment: SQL & JS performance improvements [checked in]

Here's a patch that will most likely fix this issue. It defers loading all the test info until you click on the calendar button and has a better SQL query for getting the initial data.

Comment 15

[Boris Zbarsky [:bzbarsky]]

The comment 2 url got renamed, I think.  The new correct link would be to look for is probably the "tp: ..." link on any of the talos boxes.

Any direct url I type in will just go stale after a bit, but http://graphs.mozilla.org/graph.html#show=912148 is an example, I guess.

Comment 16

[Ryan Doherty (:rdoherty)]

(In reply to comment #15)
> The comment 2 url got renamed, I think.  The new correct link would be to look
> for is probably the "tp: ..." link on any of the talos boxes.
> 
> Any direct url I type in will just go stale after a bit, but
> http://graphs.mozilla.org/graph.html#show=912148 is an example, I guess.
> 

I just tried that URL and it loads fine for me in FF 3.0.1. Not sure why it wouldn't work for you except maybe an addon?

Comment 17

[Boris Zbarsky [:bzbarsky]]

The page has changed significantly since then, from what I can tell.  Also, chances are you're using a computer that is about 10x faster than what I was using when I filed the bug.

No add-ons were involved at any point.

Comment 19

[alice nodelman [:alice] [:anode]]

Comment on attachment 334786 [details] [diff] [review]
SQL & JS performance improvements [checked in]

I got a little weirdness with empty date pickers for tests that aren't run frequently - but I think that that is something that can be shaken out after the fact.  Loading the page itself is far more important.

Comment 20

[Ryan Doherty (:rdoherty)]

in changeset 935ebf2a1651

Comment 21

[Ryan Doherty (:rdoherty)]

(In reply to comment #19)
> (From update of attachment 334786 [details] [diff] [review])
> I got a little weirdness with empty date pickers for tests that aren't run
> frequently - but I think that that is something that can be shaken out after
> the fact.  Loading the page itself is far more important.
> 

I didn't see this issue on my dev machine. Will check on graphs-stage today.

Comment 22

[Ryan Doherty (:rdoherty)]

Reopening, can't add any discrete tests to graph.

Comment 23

[Ryan Doherty (:rdoherty)]

Attachment: Fix for inability to add discrete tests to graph [checked in]

This fixes the issue I discovered yesterday. It avoids overwriting the local copy of tests by appending the data instead of re-intializing the array.

Comment 24

[alice nodelman [:alice] [:anode]]

Comment on attachment 335907 [details] [diff] [review]
Fix for inability to add discrete tests to graph [checked in]

None of this looks controversial to me.  Sorry that I didn't catch the issues with my first review.

Comment 25

[Ryan Doherty (:rdoherty)]

checked in, changeset  110:898ab5862e1f

Comment 26

[Stephen Donner [:stephend] Not actively reading bugmail]

Can someone please provide a testcase I can load/run?

As http://graphs.mozilla.org/g2/graph.html#type=series doesn't work either in production or in staging (http://graphs-staging.mozilla.org/g2/graph.html#type=series), I tried to find an equivalent URL and failed.

I am able to add discrete tests to the graph (if I understand it correctly), though that only seems to cover the last 4 comments, and not the original filing.

Thanks!

Comment 27

[Ryan Doherty (:rdoherty)]

(In reply to comment #26)
> Can someone please provide a testcase I can load/run?
> 

Sorry, the old url was still in the bug. This is the prod page:
http://graphs.mozilla.org/graph.html#type=series

staging:
http://graphs-stage.mozilla.org/graph.html#type=series

It seems like the prod server has my first patch on it which allows it to load the test names, but if you select the calendar icon and try to add multiple tests from the list, they won't load. The fix to that is on stage.

A testcase would be something like:

1) Load http://graphs-stage.mozilla.org/graph.html#type=series
2) Verify the test names load
3) Add a test via the + button
4) Add multiple tests via the + button
5) Add multiple tests via the calendar icon and selecting multiple dates in the list

Comment 28

[alice nodelman [:alice] [:anode]]

When I look at "Tp3 (RSS)" for qm-pleopard-stage01 on graphs-stage.m.o the date picker shows up empty.  The same thing happens for pretty much whatever data picker I try (for different machines/tests).

I think that there is still something wrong here.

Comment 29

[Ryan Doherty (:rdoherty)]

(In reply to comment #28)
> When I look at "Tp3 (RSS)" for qm-pleopard-stage01 on graphs-stage.m.o the date
> picker shows up empty.  The same thing happens for pretty much whatever data
> picker I try (for different machines/tests).
> 
> I think that there is still something wrong here.

It seems as though test names with parentheses in them don't work when querying the server. All other tests work fine. Will investigate.

Comment 30

[Ryan Doherty (:rdoherty)]

Attachment: Fix for testnames with parentheses [checked in]

Here's the fix for test names with parentheses. I'll attach the tests required to verify this.

Comment 31

[Ryan Doherty (:rdoherty)]

Attachment: Test data

Comment 32

[Ryan Doherty (:rdoherty)]

(In reply to comment #31)
> Created an attachment (id=336547) [details]
> Test data

Test name is Tp3 (RSS) on MacOS 10.5, qm-pleopard-stage01, 1.9 branch

Comment 33

[Ryan Doherty (:rdoherty)]

Committed, changeset 111:752632ab59eb

Comment 34

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED; consulted with Ryan (and, while I saw other bugs in the process of verification, this now works).