Last Comment Bug 634812 - Having a very large number of custom fields can make displaying show_bug.cgi slow
: Having a very large number of custom fields can make displaying show_bug.cgi ...
Status: RESOLVED FIXED
: perf
Product: Bugzilla
Classification: Server Software
Component: Creating/Changing Bugs (show other bugs)
: 4.0
: All All
: -- normal (vote)
: Bugzilla 4.0
Assigned To: Frédéric Buclin
: default-qa
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-16 19:24 PST by Wu Di
Modified: 2011-08-02 18:25 PDT (History)
6 users (show)
LpSolit: approval+
LpSolit: approval4.0+
LpSolit: blocking4.0.2+
LpSolit: blocking4.0.1-
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Query Log (91.86 KB, text/plain)
2011-02-16 19:25 PST, Wu Di
no flags Details
query_log_without_restric (41.66 KB, text/plain)
2011-02-17 00:53 PST, Wu Di
no flags Details
patch, v1 (8.15 KB, patch)
2011-07-30 18:20 PDT, Frédéric Buclin
glob: review+
Details | Diff | Review

Description Wu Di 2011-02-16 19:24:48 PST
User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11
Build Identifier: 4.0

Many duplicated queries be executed makes Bugzilla slow when show a BUG...

Reproducible: Always

Steps to Reproduce:
1. Define many custom fields.
2. Add 'Field only appears when' visible limitation to some of them.
3. Enter a BUG, many take more than 5 secs.
Actual Results:  
Enter a BUG, many take more than 5 secs.

Expected Results:  
1~2 secs.
Comment 1 Wu Di 2011-02-16 19:25:18 PST
Created attachment 513019 [details]
Query Log
Comment 2 Max Kanat-Alexander 2011-02-16 19:28:01 PST
  Thanks for the report. Could you tell me how many custom fields you have and how many of them have a "field only appears when" restriction? I need to reproduce this for myself.
Comment 3 Wu Di 2011-02-16 20:36:12 PST
87 custom fields we have.
Almost all of them have visibility restriction.
Comment 4 Max Kanat-Alexander 2011-02-16 21:28:39 PST
All right. And do you also have a lot of value restrictions? That is, "value A is only visible when field B is set to C"?
Comment 5 Wu Di 2011-02-17 00:22:27 PST
Yes, all of them have.
Comment 6 Wu Di 2011-02-17 00:53:38 PST
Created attachment 513063 [details]
query_log_without_restric

I've tried to delete all restrictions, it still very slow...
Comment 7 Max Kanat-Alexander 2011-02-17 00:55:40 PST
Are you running under mod_perl? Bugzilla is always slow unless you are running under mod_perl.
Comment 8 Wu Di 2011-02-17 01:18:14 PST
Yes, I running it under mod_perl... but it's very fast here, only except show_bug.cgi ... And I have tried to run it under cgi mode a moment ago, it's still very slow...

See '110217 16:29:45' -> '110217 16:29:49' in Attachment 513063 [details] , only 5 queries but takes 4 secs., and 100% CPU occupation(mod_perl), I have tried these 5 queries in MySQL client, only 0.022m per query...
So, I guess there must have some loops there with too many custom fields?
Comment 9 Max Kanat-Alexander 2011-02-17 01:26:48 PST
Okay, we can look into the general case of this being slow.

Were you using a previous version of Bugzilla with this many custom fields? If you were, was Bugzilla slow on that old version as well?
Comment 10 Wu Di 2011-02-17 01:38:54 PST
I used 2.0.5, 3.0.6, 3.4.1, 3.4.2, 3.4.5, 3.5.3, 3.6.1, 3,6.2,
never be that slow in my memory... just in 4.0...
May be I could downgrade to older version to retest it someday...
Comment 11 Wu Di 2011-02-17 01:43:39 PST
(In reply to comment #10)
> I used 2.0.5
3.0.5
Comment 12 Frédéric Buclin 2011-02-17 03:59:22 PST
(In reply to comment #10)
> May be I could downgrade to older version to retest it someday...

Note that you cannot downgrade Bugzilla. You would have to test on a backup of an older Bugzilla.

The good news is that 87 custom fields is pretty uncommon.
Comment 13 Max Kanat-Alexander 2011-02-17 13:52:03 PST
Wu Di: Are all your custom fields single-select fields, or are they all types? How many of each type of field do you have? (This would help me with testing.)
Comment 14 Max Kanat-Alexander 2011-02-17 14:59:39 PST
Glob, do you want to investigate this when you have some time?
Comment 15 Wu Di 2011-02-17 17:24:54 PST
(In reply to comment #13)
> Wu Di: Are all your custom fields single-select fields, or are they all types?
> How many of each type of field do you have? (This would help me with testing.)

Date/Time x 11
Drop Down x 24
Free Text x 41
Large Text Box x 11
Multiple-Selection Box x 5

(In reply to comment #14)
> Glob, do you want to investigate this when you have some time?

I will if I have some time.
Comment 16 Wu Di 2011-02-17 17:38:35 PST
We import BUG information from a CVS file which export by customer, and export a formatted Excel to them, by using a extension made by us. So it'll be many custom fields needs to be added according to the format which we decided. So, good performance with many custom fields is very important for us. :P
Comment 17 Wu Di 2011-02-17 19:11:38 PST
I deleted line: [% PROCESS "bug/field-label.html.tmpl" %]
in field.html.tmpl, problem seems be resolved (about 2 secs).
There are something wrong the field-label making...
Comment 18 Wu Di 2011-02-17 19:15:03 PST
I deleted line: [% PROCESS "bug/field-help.none.tmpl" %]
in field-lable.html.tmpl, it fast enough (about 2-3 secs).
Comment 19 Frédéric Buclin 2011-02-18 13:02:27 PST
(In reply to comment #18)
> I deleted line: [% PROCESS "bug/field-help.none.tmpl" %]
> in field-lable.html.tmpl, it fast enough (about 2-3 secs).

The reason is probably that field-label calls field-help for *each* label, which is totally useless (a single call from field.html.tmpl would be enough). The problem is twofold:

1) [% INCLUDE bug/field.html.tmpl %] from bug/edit.html.tmpl means that the data is cleared between each call to bug/field.html.tmpl (we need to use PROCESS to keep the data between calls), so there is no way to implicitly cache the content of %help_html (defined in field-help).

2) field-help loops over each field for each label. So if you have 90 custom fields, you add the ~60 hardcoded fields, and you get that the code is called (90+60)² times, i.e. 22,500 times. Remember that a single loop would be enough!!!

Confirming the bug as this is an obvious waste of ressource here, being significant or not.
Comment 20 Max Kanat-Alexander 2011-02-18 16:00:14 PST
Yeah, I realized the same thing when Wu Di explained this.

The first fix I'd try is [% RETURN IF field_help.defined %] (or whatever the variable is called) at the top of the template.
Comment 21 Max Kanat-Alexander 2011-02-18 16:00:57 PST
This may have a simple resolution that can easily be added to 4.0, so we should at least investigate this for 4.0. If the resolution requires significant refactoring, though, it will only go into 4.2.
Comment 22 Frédéric Buclin 2011-02-18 16:07:13 PST
(In reply to comment #20)
> The first fix I'd try is [% RETURN IF field_help.defined %] (or whatever the
> variable is called) at the top of the template.

This won't work as we use INCLUDE, not PROCESS. The hash is not persistent.
Comment 23 Frédéric Buclin 2011-07-30 18:20:24 PDT
Created attachment 549610 [details] [diff] [review]
patch, v1

As calling bug/field-help.none.tmpl once per field label doesn't make sense, we load it only once as we do for global/field-descs.none.tmpl. NYTProf shows a real improvement with 15 custom fields (12ms vs 238ms spent in this template).
Comment 24 Byron Jones ‹:glob› 2011-07-31 23:38:38 PDT
Comment on attachment 549610 [details] [diff] [review]
patch, v1

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

r=glob

note: this patch doesn't apply cleanly to 4.0
Comment 25 Frédéric Buclin 2011-08-01 00:46:59 PDT
(In reply to comment #24)
> note: this patch doesn't apply cleanly to 4.0

Yeah, there is a small bitrot in Bugzilla/Template.pm due to an unrelated code change. I tested, and the patch works the same way. Thanks for the review.
Comment 26 Frédéric Buclin 2011-08-01 01:41:03 PDT
If there are other useful improvements to do, we should file separate bugs for them (e.g. to decrease the time spent in bug/field-events.js.tmpl, which is also very time consuming).

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Template.pm
modified template/en/default/bug/field-help.none.tmpl
modified template/en/default/bug/field-label.html.tmpl
modified template/en/default/global/field-descs.none.tmpl
modified template/en/default/pages/fields.html.tmpl
Committed revision 7878.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Template.pm
modified template/en/default/bug/field-help.none.tmpl
modified template/en/default/bug/field-label.html.tmpl
modified template/en/default/global/field-descs.none.tmpl
modified template/en/default/pages/fields.html.tmpl
Committed revision 7629.
Comment 27 Max Kanat-Alexander 2011-08-02 17:49:29 PDT
Comment on attachment 549610 [details] [diff] [review]
patch, v1

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

::: template/en/default/bug/field-help.none.tmpl
@@ +25,2 @@
>  
> +[% RETURN UNLESS in_template_var %]

Could you simply remove all of the callers instead of adding this RETURN? There aren't that many field-help callers.

::: template/en/default/global/field-descs.none.tmpl
@@ +149,5 @@
>           IF !vars.field_descs.${bz_field.name}.defined %]
>      [% END %]
>    [% END %]
> +
> +  [% PROCESS "bug/field-help.none.tmpl" %]

Instead of adding this here, it might be better to load it in template_var when it is requested. This will add some overhead to every template_var call, as field-help is larger than all of our other stuff combined.
Comment 28 Frédéric Buclin 2011-08-02 17:59:38 PDT
(In reply to comment #27)
> > +[% RETURN UNLESS in_template_var %]
> 
> Could you simply remove all of the callers instead of adding this RETURN?
> There aren't that many field-help callers.

I already removed them. The reason I left RETURN here is to prevent future callers from calling this template, in case future reviewers or patchers are unaware of this change. It doesn't hurt.


> Instead of adding this here, it might be better to load it in template_var
> when it is requested. This will add some overhead to every template_var
> call, as field-help is larger than all of our other stuff combined.

Currently, template_var() explicitly calls field-descs.none.tmpl so it would need to be refactored, which I didn't want to do in 4.0 and 4.2 as they are closed for enhancements. Also, the most visited pages are bug reports and the search page, which all need this template. So the penalty seems pretty inexistent here. But I thought about that too. Maybe you could file a separate bug for this?
Comment 29 Max Kanat-Alexander 2011-08-02 18:25:20 PDT
(In reply to comment #28)
> I already removed them. The reason I left RETURN here is to prevent future
> callers from calling this template, in case future reviewers or patchers are
> unaware of this change. It doesn't hurt.

  Okay, I suppose that's reasonable.

>. Also, the most visited pages are bug reports and
> the search page, which all need this template. So the penalty seems pretty
> inexistent here. But I thought about that too. Maybe you could file a
> separate bug for this?

  That sounds reasonable. I suppose let's actually see if there's any problem with it before we file a bug.

Note You need to log in before you can comment on or make changes to this bug.