282145 - Bug.pm is too slow for general use

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P2)

Description

[Max Kanat-Alexander]

Apparently, it's very difficult to use Bug objects for things, because they're
slow. Developers are always complaining that use of Bug objects is slow. For
example, in the emailprefs table patch, we couldn't use Bug objects because it
slowed down BugMail 100%.

I suspect that the problem is that a Bug object needs to be "lazily
initialized." That means that we should only read data from the database at the
moment it's asked for, and not before.

I suspect I'll do a mixture of normal and lazy initialization. That is, some
fields will be immediately loaded into a bug. Other fields, that might be less
commonly used or slower, would be loaded lazily.

Comment 1

[Max Kanat-Alexander]

I did some various work on profiling creation 100, 500, and 1000 Bugs in a tight
loop (they were the first 100, 500, and 1000 bugs on landfill).

Profiling data indicates that initialization of Bug.pm spends a lot of time
hitting the database. (Duh.) If I remove all the calls after the initial SELECT
to populate the first fields, the time to init a lot of bugs becomes much smaller.

Most of that data isn't being used by most scripts anyhow. It needs to be
converted to the Bugzilla->user->groups style of caching.

Comment 2

[Max Kanat-Alexander]

Attachment: Make Bug.pm lazy

OK, here it is! I've tested it, and it worked in all my tests.

It should now be MUCH faster to init a bug,  because certain fields will only
be looked up in the database when they are asked for.

Surprisingly, not very much code was using Bugzilla::Bug.

That should definitely change after this patch is checked-in.

Comment 3

[Max Kanat-Alexander]

Oh, I forgot to mention -- I've rolled the patch from bug 281596 into this
patch. It made a lot of sense to do it now, since I was moving that whole block
of code anyway.

Comment 4

[Dave Miller [:justdave]]

THAT's what it was.  It wasn't adding Bugzilla::Bug to BugMail that caused the
slowdown (as I suggested on IRC), it was adding Bugzilla::User.  BugMail will
get a huge speed boost if we optimize Bugzilla::User initialization a bit, too.

I realized this when I saw you were lazily initializing the Bugzilla::User
objects for the role fields, then it all clicked.

Comment 5

[Dave Miller [:justdave]]

(In reply to comment #4)
> BugMail will get a huge speed boost if we optimize Bugzilla::User
> initialization a bit, too.

OK, bug 282199 filed for that.

Comment 6

[Gervase Markham [:gerv]]

Looks pretty good; I'll try and review soon.

Gerv

Comment 7

[Max Kanat-Alexander]

Cool. If you could get to it soon, that would be awesome. :-) I'd like to make
some things start using Bug objects for some of my globals-removal re-arch
stuff, but while they super-slow it just can't be done. :-)

Comment 8

[Gervase Markham [:gerv]]

Comment on attachment 174258 [details] [diff] [review]
Make Bug.pm lazy

>Index: enter_bug.cgi
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v
>retrieving revision 1.102
>diff -u -u -r1.102 enter_bug.cgi
>--- enter_bug.cgi	8 Feb 2005 16:22:25 -0000	1.102
>+++ enter_bug.cgi	14 Feb 2005 00:59:21 -0000
>@@ -355,12 +355,12 @@
> 
>     $vars->{'short_desc'}     = $cloned_bug->{'short_desc'};
>     $vars->{'bug_file_loc'}   = $cloned_bug->{'bug_file_loc'};
>-    $vars->{'keywords'}       = $cloned_bug->{'keywords'};
>+    $vars->{'keywords'}       = $cloned_bug->keywords;

Why this change?

>     $vars->{'dependson'}      = $cloned_bug_id;
>     $vars->{'blocked'}        = "";
> 
>-    if (exists $cloned_bug->{'cc'}) {
>-        $vars->{'cc'}         = join (" ", @{$cloned_bug->{'cc'}});
>+    if (defined $cloned_bug->cc) {
>+        $vars->{'cc'}         = join (" ", @{$cloned_bug->cc});

Again, "huh?" :-)

>+sub bug_id { $_[0]->{'bug_id'}; }

Not sure what this does...

BTW, can you add a comment at the top that these are in alphabetical order? It
took me a few minutes to work out why the order was so non-logical...

>+sub qa_contact () {
>+    my ($self) = @_;
>+    return $self->{'qa_contact'} if exists $self->{'qa_contact'};
>+
>+    if (Param('useqacontact') && $self->{'qa_contact_id'} > 0) {
>+        $self->{'qa_contact'} = new Bugzilla::User($self->{'qa_contact'});
>+    } else {
>+        # XXX - This is somewhat inconsistent with the asignee/reporter 

Nit: assignee.

r=gerv. Sorry for the delay.

Gerv

Comment 9

[Max Kanat-Alexander]

(In reply to comment #8)
> >-    $vars->{'keywords'}       = $cloned_bug->{'keywords'};
> >+    $vars->{'keywords'}       = $cloned_bug->keywords;
> 
> Why this change?

  Because keywords became a sub, not a field.

> >-    if (exists $cloned_bug->{'cc'}) {
> >-        $vars->{'cc'}         = join (" ", @{$cloned_bug->{'cc'}});
> >+    if (defined $cloned_bug->cc) {
> >+        $vars->{'cc'}         = join (" ", @{$cloned_bug->cc});
> 
> Again, "huh?" :-)

  Same thing. cc used to be a hash member, now it's a subroutine.

> 
> >+sub bug_id { $_[0]->{'bug_id'}; }
> 
> Not sure what this does...

  Returns $self->bug_id. It's an "accessor" for bug_id.

> BTW, can you add a comment at the top that these are in alphabetical order? It
> took me a few minutes to work out why the order was so non-logical...

  Yeah, I'll do that on checkin.

> Nit: assignee.

  Thanks. :-)

> r=gerv. Sorry for the delay.

  Hey, no problem. I'm thrilled to have the r+. :-)

Comment 10

[Max Kanat-Alexander]

Attachment: Fixed bitrot

OK, there was some kind of intense bitrot on the patch when I went to check it
in, but here's the un-bitrotted version. Carrying forward r+.

Comment 11

[Max Kanat-Alexander]

Comment on attachment 176796 [details] [diff] [review]
Fixed bitrot

Arrgh. This bitrot fix had small compilation errors, that I fixed on checkin.

Comment 12

[Max Kanat-Alexander]

Attachment: v2.1 (Actually Checked-In)

OK, this is what actually got checked-in. :-)

Comment 13

[Max Kanat-Alexander]

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.108; previous revision: 1.107
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.64; previous revision: 1.63
done