695514 - Slow performance in field-events.js.tmpl on show_bug.cgi with large number of products

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Albert Ting]

I currently have ~2000 products, ~4000 components, with no custom visibility.  Doing some profiling highlights that field-events.js.tmpl loops through all of these products/components and does "nothing".  

I don't know what the right general solution is, but in my case, I forced field-events.js.tmpl to exit if the field is "product" or "component".  Net result, what normally took 2-6 seconds to call all field.html.tmpl for show_bug.cgi now takes under 0.5 seconds.

Comment 1

[Max Kanat-Alexander]

Good to know! It might be good to file a general "Bugzilla is slow when you have many products" bug and start tracking the various issues.

Comment 2

[Frédéric Buclin]

This bug is #2 about slowness when viewing a bug.

Comment 3

[Frédéric Buclin]

Ah, I found why. field-events.js.tmpl loads data for all components, including those which have nothing to do with the bug you are viewing. It's fine to load data for all enterable products, because they are listed in the product field, but there is no need to load their components, because the component field is not updated dynamically. (For the future: once stuff is loaded dynamically, the right approach will be to use JSON-RPC to get new field values instead of loading everything at page load, especially when you know that moving bugs to another product is not the most common action).

Comment 4

[Frédéric Buclin]

Attachment: patch, v1

Here is another major performance win, when viewing a bug report (show_bug.cgi):

before: 4.21s, executing 855531 statements and 297420 subroutine calls

after: 3.60s, executing 716080 statements and 255024 subroutine calls

This 0.6 second win seems significant enough to be taken for 4.2.

Comment 5

[David Marshall]

Yahoo! has 4700 products and 35K components. I'm fairly certain we gave up on trying to generate all the stock JS a long time ago. When our recent release has stabilized, I'll look up exactly what we do.

Comment 6

[David Lawrence [:dkl]]

Comment on attachment 601269 [details] [diff] [review]
patch, v1

Review of attachment 601269 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good and indeed does give positive improvement in my testing. Nits can be fixed on checkin if you agree. r=dkl

::: template/en/default/bug/create/create.html.tmpl
@@ +231,4 @@
>        <script type="text/javascript">
>         <!--
>           [%+ INCLUDE "bug/field-events.js.tmpl" 
> +                     field = bug_fields.component product = product %]

Nit: Use commas when key/values pairs on on same line

                  field = bug_fields.component, product = product %]

::: template/en/default/bug/field.html.tmpl
@@ +133,5 @@
>          <script type="text/javascript">
>          <!--
>            initHidingOptionsForIE('[% field.name FILTER js %]');
> +          [%+ INCLUDE "bug/field-events.js.tmpl"
> +                      field = field product = bug.product_obj %]

Same Nit: Add comma

                   field = field, product = bug.product_obj %]

Comment 7

[Max Kanat-Alexander]

Comment on attachment 601269 [details] [diff] [review]
patch, v1

Review of attachment 601269 [details] [diff] [review]:
-----------------------------------------------------------------

::: template/en/default/bug/field-events.js.tmpl
@@ +21,5 @@
>    ]);
>  [% END %]
> +
> +[% legal_values = [] %]
> +[% IF field.name == "component" AND product %]

Why only specialize components here? Other per-product fields may also support being visibility controllers and value controllers in the future.

Comment 8

[Frédéric Buclin]

(In reply to Max Kanat-Alexander from comment #7)
> Why only specialize components here? Other per-product fields may also
> support being visibility controllers and value controllers in the future.

But that's for the far future IMO, and this is only relevant for components now. :) It will be easy to add other per-product fields later when they are also supported.

Comment 9

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified template/en/default/bug/field-events.js.tmpl
modified template/en/default/bug/field.html.tmpl
modified template/en/default/bug/create/create.html.tmpl
Committed revision 8136.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified template/en/default/bug/field-events.js.tmpl
modified template/en/default/bug/field.html.tmpl
modified template/en/default/bug/create/create.html.tmpl
Committed revision 8041.

Comment 10

[Max Kanat-Alexander]

(In reply to Frédéric Buclin from comment #8)
> But that's for the far future IMO, and this is only relevant for components
> now. :) It will be easy to add other per-product fields later when they are
> also supported.

  All right. FWIW, I meant Target Milestone and Version. It should be extremely simply to add support for those now that Component is supported.