mark: A photo of Mark kneeling on top of the Taal Volcano in the Philippines. It was a long hike. (Default)
Mark Smith ([staff profile] mark) wrote in [site community profile] changelog2012-04-29 10:00 pm
  • Add Memory
  • Share This Entry

[dw-free] http://bugs.dwscoalition.org/show_bug.cgi?id=4313

[commit: http://hg.dwscoalition.org/dw-free/rev/da88aac82467]

http://bugs.dwscoalition.org/show_bug.cgi?id=4313

Improve comment loading times. Object construction can be slow when you do
it hundreds to thousands of times, so this patch optimizes that by not
constructing objects unless we really need them.

Patch by [personal profile] allen.

Files modified:
  • cgi-bin/LJ/Comment.pm
  • cgi-bin/LJ/Entry.pm
  • cgi-bin/LJ/Talk.pm
--------------------------------------------------------------------------------
diff -r 3809465296ef -r da88aac82467 cgi-bin/LJ/Comment.pm
--- a/cgi-bin/LJ/Comment.pm	Sun Apr 29 23:43:12 2012 +0800
+++ b/cgi-bin/LJ/Comment.pm	Sun Apr 29 22:01:47 2012 +0000
@@ -56,10 +56,12 @@
 #    subject_orig   subject of comment w/o transcoding, present if unknown8bit
 
 #    props:   hashref of props,                    loaded if _loaded_props
+#    childids:   arrayref of child ids              loaded if _loaded_childids
 
 #    _loaded_text:   loaded talktext2 row
 #    _loaded_row:    loaded talk2 row
 #    _loaded_props:  loaded props
+#    _loaded_childids:  loaded childids
 
 my %singletons = (); # journalid->jtalkid->singleton
 
@@ -414,6 +416,19 @@
 sub children {
     my $self = $_[0];
 
+    if ( $self->{_loaded_childids} ) {
+        my @children = ();
+        my $u = $self->journal;
+        if ( $self->{childids} && scalar @{$self->{childids}} ) {
+            my @childids = @{$self->{childids}};
+            foreach my $talkid ( @childids ) {
+                my $child = LJ::Comment->new($u, jtalkid => $talkid);
+                push @children, $child;
+            }
+        }
+        return @children;
+    }
+
     my $entry = $self->entry;
     return grep { $_->{parenttalkid} == $self->{jtalkid} } $entry->comment_list;
 
diff -r 3809465296ef -r da88aac82467 cgi-bin/LJ/Entry.pm
--- a/cgi-bin/LJ/Entry.pm	Sun Apr 29 23:43:12 2012 +0800
+++ b/cgi-bin/LJ/Entry.pm	Sun Apr 29 22:01:47 2012 +0000
@@ -48,6 +48,7 @@
 #    allowmask:  if _loaded_row
 #    posterid:   if _loaded_row
 #    comments:   arrayref of comment objects on this entry
+#    talkdata:   hash of raw comment data for this entry
 
 #    userpic
 
@@ -55,6 +56,7 @@
 #    _loaded_row:      loaded log2 row
 #    _loaded_props:    loaded props
 #    _loaded_comments: loaded comments
+#    _loaded_talkdata: loaded talkdata
 
 my %singletons = (); # journalid->jitemid->singleton
 
@@ -484,17 +486,31 @@
     return 1 if $self->{_loaded_comments};
 
     # need to load using talklib API
-    my $comment_ref = LJ::Talk::get_talk_data($self->journal, 'L', $self->jitemid);
+    my $comment_ref =  $self->{_loaded_talkdata} ? $self->{talkdata} : LJ::Talk::get_talk_data($self->journal, 'L', $self->jitemid);
+
     die "unable to load comment data for entry"
         unless ref $comment_ref;
 
+    my @comment_list;
+
+    my $u = $self->journal;
+    my $nodeid = $self->jitemid;
+
     # instantiate LJ::Comment singletons and set them on our $self
-    # -- members were filled in to the LJ::Comment singleton during the talklib call,
-    #    so we'll just re-instantiate here and rely on the fact that the singletons
-    #    already exist and have db rows absorbed into them
-    $self->set_comment_list
-        ( map { LJ::Comment->new( $self->journal, jtalkid => $_->{talkid}) }
-          values %$comment_ref );
+    foreach my $jtalkid ( keys %$comment_ref ) {
+        my $row = $comment_ref->{$jtalkid};
+        # at this point we have data for this comment loaded in memory
+        # -- instantiate an LJ::Comment object as a singleton and absorb
+        #    that data into the object
+        my $comment = LJ::Comment->new($u, jtalkid => $jtalkid);
+        # add important info to row
+        $row->{nodetype} = "L";
+        $row->{nodeid}   = $nodeid;
+        $comment->absorb_row(%$row);
+        
+        push @comment_list, $comment;
+    }        
+    $self->set_comment_list( @comment_list );
 
     return $self;
 }
@@ -514,6 +530,15 @@
     return 1;
 }
 
+sub set_talkdata {
+    my ( $self, $talkdata ) = @_;
+
+    $self->{talkdata} = $talkdata;
+    $self->{_loaded_talkdata} = 1;
+
+    return 1;
+}
+
 sub reply_count {
     my $self = $_[0];
     my $rc = $self->prop('replycount');
diff -r 3809465296ef -r da88aac82467 cgi-bin/LJ/Talk.pm
--- a/cgi-bin/LJ/Talk.pm	Sun Apr 29 23:43:12 2012 +0800
+++ b/cgi-bin/LJ/Talk.pm	Sun Apr 29 22:01:47 2012 +0000
@@ -716,39 +716,12 @@
         }
     };
 
-    my $make_comment_singleton = sub {
-        my ($jtalkid, $row) = @_;
-        return 1 unless $nodetype eq 'L';
-
-        # at this point we have data for this comment loaded in memory
-        # -- instantiate an LJ::Comment object as a singleton and absorb
-        #    that data into the object
-        my $comment = LJ::Comment->new($u, jtalkid => $jtalkid);
-        # add important info to row
-        $row->{nodetype} = $nodetype;
-        $row->{nodeid}   = $nodeid;
-        $comment->absorb_row(%$row);
-
-        return 1;
-    };
-
-    # This is a bit tricky.  Since we just loaded and instantiated all comment singletons for this
-    # entry (journalid, jitemid) we'll instantiate a skeleton LJ::Entry object (probably a singleton
-    # used later in this request anyway) and let it know that its comments are already loaded
+    # Save the talkdata on the entry for later
     my $set_entry_cache = sub {
         return 1 unless $nodetype eq 'L';
 
-        my $entry = LJ::Entry->new($u, jitemid => $nodeid);
-
-        # find all singletons that LJ::Comment knows about, then grep for the ones we've set in
-        # this get_talk_data call (ones for this userid / nodeid)
-        # note: This is a heavily-used path. The hash lookup on nodeid is a deliberate optimization
-        my $uid = $u->userid;
-        LJ::Comment->preload_rows();
-        my @comments_for_entry =
-            grep { $_->{journalid} == $uid && $_->{nodeid} == $nodeid } LJ::Comment->all_singletons;
-
-        $entry->set_comment_list(@comments_for_entry);
+        my $entry = LJ::Entry->new( $u, jitemid => $nodeid );
+        $entry->set_talkdata( $ret );
     };
 
     my $memcache_good = sub {
@@ -771,9 +744,6 @@
                 parenttalkid => $par,
             };
 
-            # instantiate comment singleton
-            $make_comment_singleton->($talkid, $ret->{$talkid});
-
             # comments are counted if they're 'A'pproved or 'F'rozen
             $rp_ourcount++ if $state eq "A" || $state eq "F";
         }
@@ -820,9 +790,6 @@
             $row_arg{nodeid}   = $nodeid;
             $row_arg{nodetype} = $nodetype;
 
-            # instantiate comment singleton
-            $make_comment_singleton->($r->{talkid}, \%row_arg);
-
             # set talk2row memcache key for this bit of data
             LJ::Talk::add_talk2row_memcache($u->id, $r->{talkid}, \%row_arg);
         }
@@ -1021,15 +988,15 @@
                         ( $remote->userid == $uposterid || # made in remote's journal
                           $remote->userid == $post->{posterid} || # made by remote
                           $remote->can_manage( $u ) || # made in a journal remote manages
-                           (
-                              # remote authored the parent, and this comment is by an admin
-                              exists $posts->{$parenttalkid} &&
-                              $posts->{$parenttalkid}->{posterid} &&
-                              $posts->{$parenttalkid}->{posterid} == $remote->userid &&
-                              $poster && $poster->can_manage( $u )
-                           )
+                          (
+                           # remote authored the parent, and this comment is by an admin
+                           exists $posts->{$parenttalkid} &&
+                           $posts->{$parenttalkid}->{posterid} &&
+                           $posts->{$parenttalkid}->{posterid} == $remote->userid &&
+                           $poster && $poster->can_manage( $u )
+                          )
                         )
-                      );
+                    );
             }
             $post->{'_show'} = $should_show;
             $post_count += $should_show;
@@ -1328,6 +1295,36 @@
             load_userpics( $opts->{userpicref}, \@load_pic );
         }
     }
+
+    # make singletons for the returned comments
+    my $make_comment_singleton = sub {
+        my ($self, $jtalkid, $row) = @_;
+        return 1 unless $nodetype eq 'L';
+
+        # at this point we have data for this comment loaded in memory
+        # -- instantiate an LJ::Comment object as a singleton and absorb
+        #    that data into the object
+        my $comment = LJ::Comment->new($u, jtalkid => $jtalkid);
+        # add important info to row
+        $row->{nodetype} = $nodetype;
+        $row->{nodeid}   = $nodeid;
+        $comment->absorb_row(%$row);
+
+        $comment->{childids} = $row->{children};
+        $comment->{_loaded_childids} = 1;
+
+        if ( $row->{children} && scalar @{$row->{children}} ) {
+            foreach my $child ( @{$row->{children}} ) {
+                $self->($self, $child, $posts->{$child});
+            }
+        }
+        return 1;
+    };
+
+    foreach my $talkid ( @top_replies ) {
+        $make_comment_singleton->($make_comment_singleton, $talkid, $posts->{$talkid});
+    }
+
     return map { $posts->{$_} } @top_replies;
 }
 
--------------------------------------------------------------------------------
  • Add Memory
  • Share This Entry
(0 comments)

Post a comment in response:

This account has disabled anonymous posting.
If you don't have an account you can create one now.
HTML doesn't work in the subject.
More info about formatting

If you are unable to use this captcha for any reason, please contact us by email at support@dreamwidth.org

 
Notice: This account is set to log the IP addresses of everyone who comments.
Links will be displayed as unclickable URLs to help prevent spam.