415652 - Implement Bugzilla->active_custom_fields

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect, P1)

Description

[Frédéric Buclin]

Every time you access a bug, Bugzilla::Object::_init() is called. But this method calls my $columns = join(',', $class->DB_COLUMNS); which in turn calls:

sub DB_COLUMNS {
    my $dbh = Bugzilla->dbh;
    my @custom = Bugzilla->get_fields({ custom => 1, obsolete => 0});

Even Bugzilla::Bug::AUTOLOAD calls get_fields() again via:

  if (!_validate_attribute($attr)) {

and via:

      $self->{_multi_selects} ||= [Bugzilla->get_fields(
          {custom => 1, type => FIELD_TYPE_MULTI_SELECT })];

It appears very clearly that the most often used criteria is:

  { custom=>1, obsolete=>0 }

So we should either have a separate Bugzilla->get_active_custom_fields() which would cache this list of fields, or have Bugzilla->get_fields() cache all fields and do the matching itself instead of letting the DB do it. On my slow installation, especially the DB, this is a real perf issue. A simple warn() in Bugzilla->get_fields() shows how often this method is called, meaning how often the DB is reached... for nothing.

Comment 1

[Max Kanat-Alexander]

In actuality this is not a performance problem at all for most installations, due to the DB's query cache, but yes, it should definitely be handled for architectural reasons. However, it is not a blocker.

I'd be OK with get_active_custom_fields for now--that sounds like probably the easiest solution, though not the best. I'd like to come up with some generally better way to cache the calls to get_fields. Although instead, we could easily cache the result of Bugzilla::Bug::DB_COLUMNS as Bugzilla->request_cache->{bug_db_columns}.

Comment 2

[Frédéric Buclin]

Attachment: patch, v1

Implement Bugzilla->get_active_custom_fields and remove the no-longer used Bugzilla->custom_field_names method.

Comment 3

[Frédéric Buclin]

I'm restricting this bug to the implementation of Bugzilla->get_active_custom_fields, which is by far the most requested query. bkor, could you test what the benefit of this patch is?

Comment 4

[Max Kanat-Alexander]

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

This *looks* fine. I'd like you to change the name from "get_active_custom_fields" to just "active_custom_fields", though.

Comment 5

[Frédéric Buclin]

Max, do you want bkor to do some perf tests on it or a review or can I check it in?

Comment 6

[Max Kanat-Alexander]

Hmm. I didn't actually test it, that was my only concern. If you say it works and that you've tested it pretty well, I trust you.

Will you do the change to active_custom_fields on checkin?

Comment 7

[Frédéric Buclin]

(In reply to comment #6)
> Will you do the change to active_custom_fields on checkin?

Yes, as you requested it in your review.

Comment 8

[Frédéric Buclin]

Checking in Bugzilla.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla.pm,v  <--  Bugzilla.pm
new revision: 1.64; previous revision: 1.63
done
Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.371; previous revision: 1.370
done
Checking in colchange.cgi;
/cvsroot/mozilla/webtools/bugzilla/colchange.cgi,v  <--  colchange.cgi
new revision: 1.62; previous revision: 1.61
done
Checking in config.cgi;
/cvsroot/mozilla/webtools/bugzilla/config.cgi,v  <--  config.cgi
new revision: 1.28; previous revision: 1.27
done
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.160; previous revision: 1.159
done
Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.78; previous revision: 1.77
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.196; previous revision: 1.195
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.406; previous revision: 1.405
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.234; previous revision: 1.233
done
Checking in Bugzilla/Field.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Field.pm,v  <--  Field.pm
new revision: 1.31; previous revision: 1.30
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.114; previous revision: 1.113
done
Checking in template/en/default/bug/show-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show-multiple.html.tmpl,v  <--  show-multiple.html.tmpl
new revision: 1.41; previous revision: 1.40
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.82; previous revision: 1.81
done
Checking in template/en/default/list/edit-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v  <--  edit-multiple.html.tmpl
new revision: 1.48; previous revision: 1.47
done