If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Bug.pm is too slow for general use

RESOLVED FIXED in Bugzilla 2.20

Status

()

Bugzilla
Bugzilla-General
P2
normal
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

(Blocks: 1 bug)

2.19.2
Bugzilla 2.20
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

13 years ago
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.
(Assignee)

Comment 1

13 years ago
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.
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.20
(Assignee)

Comment 2

13 years ago
Created attachment 174258 [details] [diff] [review]
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.
Attachment #174258 - Flags: review?
(Assignee)

Comment 3

13 years ago
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.
Depends on: 281596
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.
(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.
(Assignee)

Updated

13 years ago
Attachment #174258 - Flags: review? → review?(gerv)
Looks pretty good; I'll try and review soon.

Gerv
(Assignee)

Updated

13 years ago
Priority: -- → P2
(Assignee)

Comment 7

13 years ago
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. :-)
(Assignee)

Updated

13 years ago
Attachment #174258 - Flags: review?(gerv) → review?(bugzilla)
(Assignee)

Updated

13 years ago
Assignee: mkanat → nobody
Status: ASSIGNED → NEW
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
Target Milestone: Bugzilla 2.22 → Bugzilla 2.20
(Assignee)

Updated

13 years ago
Assignee: nobody → mkanat
Status: ASSIGNED → NEW
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
Attachment #174258 - Flags: review?(bugzilla) → review+
(Assignee)

Comment 9

13 years ago
(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+. :-)
Status: NEW → ASSIGNED
Flags: approval?
Flags: approval? → approval+
(Assignee)

Comment 10

13 years ago
Created attachment 176796 [details] [diff] [review]
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+.
(Assignee)

Updated

13 years ago
Attachment #174258 - Attachment is obsolete: true
Attachment #176796 - Flags: review+
(Assignee)

Comment 11

13 years ago
Comment on attachment 176796 [details] [diff] [review]
Fixed bitrot

Arrgh. This bitrot fix had small compilation errors, that I fixed on checkin.
Attachment #176796 - Attachment is obsolete: true
(Assignee)

Comment 12

13 years ago
Created attachment 176797 [details] [diff] [review]
v2.1 (Actually Checked-In)

OK, this is what actually got checked-in. :-)
Attachment #176797 - Flags: review+
(Assignee)

Comment 13

13 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Assignee)

Updated

13 years ago
Blocks: 285541
You need to log in before you can comment on or make changes to this bug.