Bug 909360 - backport upstream bug 898830 to bmo/4.2 for performance improvement in show_bug.cgi

3 files changed, 45 insertions, 42 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 84bcedbd5..a544ed614 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -22,10 +22,11 @@
 #                 Marc Schumann <wurblzap@gmail.com>
 #                 Frédéric Buclin <LpSolit@gmail.com>
 
-use strict;
-
 package Bugzilla::Attachment;
 
+use 5.10.0;
+use strict;
+
 =head1 NAME
 
 Bugzilla::Attachment - Bugzilla attachment class.
@@ -142,8 +143,7 @@ the ID of the bug to which the attachment is attached
 =cut
 
 sub bug_id {
-    my $self = shift;
-    return $self->{bug_id};
+    return $_[0]->{bug_id};
 }
 
 =over
@@ -157,11 +157,8 @@ the bug object to which the attachment is attached
 =cut
 
 sub bug {
-    my $self = shift;
-
     require Bugzilla::Bug;
-    $self->{bug} ||= Bugzilla::Bug->new({ id => $self->bug_id, cache => 1 });
-    return $self->{bug};
+    return $_[0]->{bug} //= Bugzilla::Bug->new({ id => $_[0]->bug_id, cache => 1 });
 }
 
 =over
@@ -175,8 +172,7 @@ user-provided text describing the attachment
 =cut
 
 sub description {
-    my $self = shift;
-    return $self->{description};
+    return $_[0]->{description};
 }
 
 =over
@@ -190,8 +186,7 @@ the attachment's MIME media type
 =cut
 
 sub contenttype {
-    my $self = shift;
-    return $self->{mimetype};
+    return $_[0]->{mimetype};
 }
 
 =over
@@ -205,9 +200,8 @@ the user who attached the attachment
 =cut
 
 sub attacher {
-    my $self = shift;
-    return $self->{attacher}
-        ||= new Bugzilla::User({ id => $self->{submitter_id}, cache => 1 });
+    return $_[0]->{attacher}
+        //= new Bugzilla::User({ id => $_[0]->{submitter_id}, cache => 1 });
 }
 
 =over
@@ -221,8 +215,7 @@ the date and time on which the attacher attached the attachment
 =cut
 
 sub attached {
-    my $self = shift;
-    return $self->{creation_ts};
+    return $_[0]->{creation_ts};
 }
 
 =over
@@ -236,8 +229,7 @@ the date and time on which the attachment was last modified.
 =cut
 
 sub modification_time {
-    my $self = shift;
-    return $self->{modification_time};
+    return $_[0]->{modification_time};
 }
 
 =over
@@ -251,8 +243,7 @@ the name of the file the attacher attached
 =cut
 
 sub filename {
-    my $self = shift;
-    return $self->{filename};
+    return $_[0]->{filename};
 }
 
 =over
@@ -266,8 +257,7 @@ whether or not the attachment is a patch
 =cut
 
 sub ispatch {
-    my $self = shift;
-    return $self->{ispatch};
+    return $_[0]->{ispatch};
 }
 
 =over
@@ -281,8 +271,7 @@ whether or not the attachment is obsolete
 =cut
 
 sub isobsolete {
-    my $self = shift;
-    return $self->{isobsolete};
+    return $_[0]->{isobsolete};
 }
 
 =over
@@ -296,8 +285,7 @@ whether or not the attachment is private
 =cut
 
 sub isprivate {
-    my $self = shift;
-    return $self->{isprivate};
+    return $_[0]->{isprivate};
 }
 
 =over
@@ -314,8 +302,7 @@ matches, because this will return a value even if it's matched by the generic
 =cut
 
 sub is_viewable {
-    my $self = shift;
-    my $contenttype = $self->contenttype;
+    my $contenttype = $_[0]->contenttype;
     my $cgi = Bugzilla->cgi;
 
     # We assume we can view all text and image types.
@@ -389,7 +376,7 @@ the length (in characters) of the attachment content
 
 sub datasize {
     my $self = shift;
-    return $self->{datasize} if exists $self->{datasize};
+    return $self->{datasize} if defined $self->{datasize};
 
     # If we have already retrieved the data, return its size.
     return length($self->{data}) if exists $self->{data};
@@ -479,11 +466,8 @@ flags that have been set on the attachment
 =cut
 
 sub flags {
-    my $self = shift;
-
     # Don't cache it as it must be in sync with ->flag_types.
-    $self->{flags} = [map { @{$_->{flags}} } @{$self->flag_types}];
-    return $self->{flags};
+    return $_[0]->{flags} = [map { @{$_->{flags}} } @{$_[0]->flag_types}];
 }
 
 =over
@@ -507,8 +491,7 @@ sub flag_types {
                  attach_id    => $self->id, 
                  active_or_has_flags => $self->bug_id };
 
-    $self->{flag_types} = Bugzilla::Flag->_flag_types($vars);
-    return $self->{flag_types};
+    return $self->{flag_types} = Bugzilla::Flag->_flag_types($vars);
 }
 
 ###############################
@@ -620,7 +603,7 @@ sub _check_filename {
         else {
             ThrowUserError('file_not_specified');
         }
-    }
+   }
 
     # Remove path info (if any) from the file name.  The browser should do this
     # for us, but some are buggy.  This may not work on Mac file names and could
@@ -703,6 +686,24 @@ sub get_attachments_by_bug {
 
         push(@{$att{$_->attach_id}->{flags}}, $_) foreach @$flags;
         $attachments = [sort {$a->id <=> $b->id} values %att];
+
+        # Preload attachers.
+        my %user_ids = map { $_->{submitter_id} => 1 } @$attachments;
+        my $users = Bugzilla::User->new_from_list([keys %user_ids]);
+        my %user_map = map { $_->id => $_ } @$users;
+        foreach my $attachment (@$attachments) {
+            $attachment->{attacher} = $user_map{$attachment->{submitter_id}};
+        }
+
+        # Preload datasizes.
+        my $sizes =
+          $dbh->selectall_hashref('SELECT attach_id, LENGTH(thedata) AS size
+                                   FROM attachments LEFT JOIN attach_data ON attach_id = id
+                                   WHERE bug_id = ?',
+                                   'attach_id', undef, $bug_id);
+
+        # Force the size of attachments not in the DB to be recalculated.
+        $_->{datasize} = $sizes->{$_->id}->{size} || undef foreach @$attachments;
     }
     return $attachments;
 }
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 87714011c..946fe8cb1 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -831,9 +831,7 @@ sub in_group_id {
 # besides the name of the commenter.
 sub groups_with_icon {
     my $self = shift;
-
-    my @groups = grep { $_->icon_url } @{ $self->direct_group_membership };
-    return \@groups;
+    return $self->{groups_with_icon} //= [grep { $_->icon_url } @{ $self->direct_group_membership }];
 }
 
 sub get_products_by_permission {
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index e12882215..96dad8327 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -28,6 +28,7 @@
 
 package Bugzilla::Util;
 
+use 5.10.1;
 use strict;
 
 use base qw(Exporter);
@@ -93,7 +94,10 @@ sub html_quote {
     $var =~ s/"/&quot;/g;
     # Obscure '@'.
     $var =~ s/\@/\&#64;/g;
-    if (Bugzilla->params->{'utf8'}) {
+
+    state $use_utf8 = Bugzilla->params->{'utf8'};
+
+    if ($use_utf8) {
         # Remove the following characters because they're
         # influencing BiDi:
         # --------------------------------------------------------
@@ -115,7 +119,7 @@ sub html_quote {
         # |U+200e|Left-To-Right Mark        |0xe2 0x80 0x8e      |
         # |U+200f|Right-To-Left Mark        |0xe2 0x80 0x8f      |
         # --------------------------------------------------------
-        $var =~ s/[\x{202a}-\x{202e}]//g;
+        $var =~ tr/\x{202a}-\x{202e}//d;
     }
     return $var;
 }