576670 - Optimize Search.pm's "init" method for being called many times in a loop

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, enhancement)

Description

[Max Kanat-Alexander]

So, I'm working on the "bz-search-test" bug, and when it's done, it's going to run about 60 million tests. A lot of these tests involve creating a Bugzilla::Search object, and it turns out that since we don't do this many times in a loop inside of Bugzilla, it's never been optimized very well. There are a lot of things to optimize that my profiler shows up.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Okay, this makes my search test run 3x-4x faster. Here's an explanation of the optimizations:

* Bugzilla->get_fields was the largest time-taker in init(). Bugzilla->fields is a better get_fields that does caching. In the future, it can be enhanced to accept more of the arguments that get_fields takes. It also optionally returns things as a hash, since even doing "map { $_->id => $_ }" on fields was showing up as a significant time-taker in my profiles.

* Calling $template->process in Bugzilla::Error was slow. Tests really don't care about error text. In fact, they'd rather not know it, because they don't want the localized version, if they're going to check something about the error. So I added a new error mode for xt/search.t, called ERROR_MODE_TEST, that just dumps all the error information.

* Generating the Traceback for errors was slow, so now we only do it if we're a CodeError and we're actually going to be using the traceback.

* Calling $user->groups_as_string was slow, because of the "map { $_->id }" on group objects. So now I cache the group ids separately.

* I noticed that bz_locations was getting called a lot, and it's really a constant, so I Memoize'd it. This is safe to do because its contents will never change, even under mod_perl.

* There's a || 0 in one place to stop a warning that happens when passing in an invalid value.

* %chart_fields in init() now just has the contents of Bugzilla->fields, and I pass that along and use the objects in the search functions.

* Calling login_to_id over and over for the same few users was slow, so I cache its info now.

Comment 2

[Max Kanat-Alexander]

I'm going to re-work this patch slightly so that the test can go in first, and then this. That way, the test can still make 4.0, and the optimizations will make 4.2.

Comment 3

[Max Kanat-Alexander]

Attachment: v2

Since this now depends on the search test (and the search test proves that it works just fine), I can grant myself review as the Search.pm owner for 4.2 and check it in once the test is checked in.

Comment 4

[:glob ✱]

Comment on attachment 455975 [details] [diff] [review]
v2

r=glob

Comment 5

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla.pm
modified Bugzilla/Constants.pm
modified Bugzilla/Error.pm
modified Bugzilla/Field.pm
modified Bugzilla/Search.pm
modified Bugzilla/Template.pm
modified Bugzilla/User.pm
modified xt/lib/Bugzilla/Test/Search.pm
Committed revision 7314.

Comment 6

[Max Kanat-Alexander]

Attachment: Checked-In Version

Here's what got checked in--I had to fix a bit of bitrot on checkin. Carrying forward r+.