[dw-free] optimize viewing entries with large # of comments

[fu]

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

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

Efficiency tweaks! Quick summary:

* now only load the information on the comments we need

* make sure we don't end up looking through our hashes over and over again
trying to find unloaded singletons that were actually already loaded.

Implementation Details:
* creates a new sub "preload_props" which takes a list of comments to preload
properties for, so we can load the info for only those comments that we want
to display, rather than trying to preload props for (effectively) every
comment on the entry

* changes the lists of unloaded items into hashes, to make it easy to mark
as loaded. Note: we have this additional data structure so that we don't
need to iterate over all comment singletons in order to find those that
haven't been loaded yet

* make sure we remove from the unloaded list when we absorb_row. Important
because in LJ::Talk::get_talk_data, we create a comment singleton and call
absorb_row for *every* comment in the entry (but have it in the unloaded
data structure, so we iterate over it every time we call preload_rows -- and
we call that a lot)
* clears out the unloaded items hashes when we reset_singletons to avoid a
potential memory leak

Patch by allen.

Files modified:

• cgi-bin/LJ/Comment.pm
• cgi-bin/LJ/Talk.pm

--------------------------------------------------------------------------------
diff -r 6121fc84a811 -r f18a58c7d31f cgi-bin/LJ/Comment.pm
--- a/cgi-bin/LJ/Comment.pm	Tue Feb 14 21:54:25 2012 +0800
+++ b/cgi-bin/LJ/Comment.pm	Tue Feb 14 22:15:12 2012 +0800
@@ -63,15 +63,18 @@
 
 my %singletons = (); # journalid->jtalkid->singleton
 
+# singletons still to be loaded
+my %unloaded_singletons = ();
+my %unloaded_text_singletons = ();
+my %unloaded_prop_singletons = ();
+
 sub reset_singletons {
     %singletons = ();
+    %unloaded_singletons = ();
+    %unloaded_text_singletons = ();
+    %unloaded_prop_singletons = ();
 }
 
-# singletons still to be loaded
-my @unloaded_singletons;
-my @unloaded_text_singletons;
-my @unloaded_prop_singletons;
-
 # <LJFUNC>
 # name: LJ::Comment::new
 # class: comment
@@ -103,9 +106,9 @@
         jtalkid => $jtalkid,
     }, $class;
 
-    push @unloaded_singletons, $self;
-    push @unloaded_text_singletons, $self;
-    push @unloaded_prop_singletons, $self;
+    $unloaded_singletons{$self->singletonkey} = $self;
+    $unloaded_text_singletons{$self->singletonkey} = $self;
+    $unloaded_prop_singletons{$self->singletonkey} = $self;
 
     return $singletons{$journalid}->{$jtalkid} = $self;
 }
@@ -220,6 +223,7 @@
 
     $self->{$_} = $row{$_} foreach (qw(nodetype nodeid parenttalkid posterid datepost state));
     $self->{_loaded_row} = 1;
+    delete $unloaded_singletons{$self->singletonkey};
 }
 
 sub url {
@@ -310,6 +314,10 @@
     return $_[0]->{journalid};
 }
 
+sub singletonkey {
+    return $_[0]->{journalid} . "-" . $_[0]->{jtalkid};
+}
+
 # return LJ::Entry of entry comment is in, or undef if it's not
 # a nodetype of L
 sub entry {
@@ -460,10 +468,8 @@
 
 # class method:
 sub preload_rows {
-    my @to_load =
-        map  { [ $_->journal, $_->jtalkid ] }
-            grep { ! $_->{_loaded_row} } @unloaded_singletons;
-
+    my @to_load = ();
+    push @to_load, map  { [ $_->journal, $_->jtalkid ] } values %unloaded_singletons;
     # already loaded?
     return 1 unless @to_load;
 
@@ -473,7 +479,7 @@
     # make a mapping of journalid-jtalkid => $row
     my %row_map = map { join("-", $_->{journalid}, $_->{jtalkid}) => $_ } @rows;
 
-    foreach my $obj (@unloaded_singletons) {
+    foreach my $obj (values %unloaded_singletons) {
         my $u = $obj->journal;
 
         my $row = $row_map{join("-", $u->id, $obj->jtalkid)};
@@ -483,7 +489,7 @@
         $obj->absorb_row(%$row);
     }
 
-    @unloaded_singletons = ();
+    %unloaded_singletons = ();
     return 1;
 }
 
@@ -498,15 +504,12 @@
     my $entry_uid = $entryu->id;
 
     # find singletons which don't already have text loaded
-    my (@to_load, @to_keep);
-    foreach my $c_obj (@unloaded_text_singletons) {
+    my @to_load;
+    foreach my $c_obj (values %unloaded_text_singletons) {
         if ($c_obj->journalid == $entry_uid) {
             push @to_load, $c_obj;
-        } else {
-            push @to_keep, $c_obj;
         }
     }
-    @unloaded_text_singletons = @to_keep;
 
     my $ret  = LJ::get_talktext2($entryu, map { $_->jtalkid } @to_load);
     return 0 unless $ret && ref $ret;
@@ -530,6 +533,7 @@
         }
 
         $c_obj->{_loaded_text} = 1;
+        delete $unloaded_text_singletons{$self->singletonkey};
     }
 
     return 1;
@@ -602,7 +606,7 @@
     } else {
         $self->{$_} = undef foreach qw(subject body);
         $self->{_loaded_text} = 0;
-        push @unloaded_text_singletons, $self;
+        $unloaded_text_singletons{$self->singletonkey} = $self;
     }
     # otherwise _loaded_text=0 and we won't do any optimizations
 
@@ -659,22 +663,9 @@
     return $self->{props} || {};
 }
 
-sub _load_props {
-    my $self = $_[0];
-    return 1 if $self->{_loaded_props};
-
-    my $journalid = $self->journalid;
-
-    # find singletons which don't already have props loaded
-    my (@to_load, @to_keep);
-    foreach my $c_obj (@unloaded_prop_singletons) {
-        if ($c_obj->journalid == $journalid) {
-            push @to_load, $c_obj;
-        } else {
-            push @to_keep, $c_obj;
-        }
-    }
-    @unloaded_prop_singletons = @to_keep;
+# class method:  preloads the props on the provided list of Comment objects.
+sub preload_props {
+    my ($class, $journalid, @to_load) = @_;
 
     my $prop_ret = {};
     LJ::load_talk_props2($journalid, [ map { $_->jtalkid } @to_load ], $prop_ret);
@@ -683,6 +674,34 @@
     foreach my $c_obj (@to_load) {
         $c_obj->{props} = $prop_ret->{$c_obj->jtalkid} || {};
         $c_obj->{_loaded_props} = 1;
+        delete $unloaded_prop_singletons{$c_obj->singletonkey};
+    }
+
+    return 1;
+}
+
+sub _load_props {
+    my $self = $_[0];
+    return 1 if $self->{_loaded_props};
+
+    my $journalid = $self->journalid;
+
+    # find singletons which don't already have props loaded
+    my @to_load;
+    foreach my $c_obj (values %unloaded_prop_singletons) {
+        if ($c_obj->journalid == $journalid) {
+            push @to_load, $c_obj;
+        }
+    }
+
+    my $prop_ret = {};
+    LJ::load_talk_props2($journalid, [ map { $_->jtalkid } @to_load ], $prop_ret);
+
+    # iterate over comment objects to load and fill in their props members
+    foreach my $c_obj (@to_load) {
+        $c_obj->{props} = $prop_ret->{$c_obj->jtalkid} || {};
+        $c_obj->{_loaded_props} = 1;
+        delete $unloaded_prop_singletons{$c_obj->singletonkey};
     }
 
     return 1;
diff -r 6121fc84a811 -r f18a58c7d31f cgi-bin/LJ/Talk.pm
--- a/cgi-bin/LJ/Talk.pm	Tue Feb 14 21:54:25 2012 +0800
+++ b/cgi-bin/LJ/Talk.pm	Tue Feb 14 22:15:12 2012 +0800
@@ -1219,6 +1219,16 @@
     my ($posts_loaded, $subjects_loaded);
     $posts_loaded = LJ::get_talktext2($u, @posts_to_load);
     $subjects_loaded = LJ::get_talktext2($u, {'onlysubjects'=>1}, @subjects_to_load) if @subjects_to_load;
+
+    # preload props
+    my @ids_to_preload = @posts_to_load;
+    push @ids_to_preload, @subjects_to_load;
+    my @to_preload = ();
+    foreach my $jtalkid (@ids_to_preload) {
+        push @to_preload, LJ::Comment->new($u, jtalkid => $jtalkid);
+    }
+    LJ::Comment->preload_props($u, @to_preload);
+
     foreach my $talkid (@posts_to_load) {
         if ( $opts->{'top-only'} ) {
             $posts->{$talkid}->{'hide_children'} = 1;
--------------------------------------------------------------------------------