Closed Bug 282145 Opened 20 years ago Closed 20 years ago

Bug.pm is too slow for general use

Categories

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

2.19.2
defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.20

People

(Reporter: mkanat, Assigned: mkanat)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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.
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.
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.20
Attached patch Make Bug.pm lazy (obsolete) (deleted) — Splinter Review
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?
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.
Attachment #174258 - Flags: review? → review?(gerv)
Looks pretty good; I'll try and review soon. Gerv
Priority: -- → P2
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. :-)
Attachment #174258 - Flags: review?(gerv) → review?(bugzilla)
Assignee: mkanat → nobody
Status: ASSIGNED → NEW
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Status: NEW → ASSIGNED
Target Milestone: Bugzilla 2.22 → Bugzilla 2.20
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+
(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+
Attached patch Fixed bitrot (obsolete) (deleted) — Splinter Review
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+.
Attachment #174258 - Attachment is obsolete: true
Attachment #176796 - Flags: review+
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
Attached patch v2.1 (Actually Checked-In) (deleted) — Splinter Review
OK, this is what actually got checked-in. :-)
Attachment #176797 - Flags: review+
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
Closed: 20 years ago
Resolution: --- → FIXED
Blocks: 285541
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: