Reimplement parameter localization using a goto loop
Aaron Crane [Wed, 16 Dec 2015 14:39:49 +0000 (14:39 +0000)]
The previous recursive implementation is slow, and leads to cluttered stack
traces. No straightforward looping approaches work. Using a goto is not
exactly pleasant, but it is at least confined to a single method, and has no
other downsides.

lib/DBIx/Class/ResultSet/ParameterizedJoinHack.pm

index 53a04b0..6097215 100644 (file)
@@ -37,22 +37,30 @@ sub with_parameterized_join {
   );
 }
 
-sub _localize_parameters {
-  my ($self, $final, $params, $store, $first, @rest) = @_;
-  return $final->() unless $first;
-  local $store->{$first}{params} = $params->{$first};
-  $self->_localize_parameters($final, $params, $store, @rest);
-}
-
 sub call_with_parameters {
   my ($self, $method, @args) = @_;
-  my %params = %{$self->{attrs}{join_parameters}||{}};
+  my $params = $self->{attrs}{join_parameters}||{};
+  my @param_names = keys %$params;
   my $store = $self->_parameterized_join_store;
-  return $self->_localize_parameters(
-    sub { $self->$method(@args) },
-    \%params, $store,
-    keys %params
-  );
+  # This loop is implemented using "goto"; this means the "local" doesn't have
+  # to be in a separate scope, which would defeat the point. Nor does "local
+  # ... foreach ..." work; the localization doesn't survive the loop. The
+  # alternative would be to use recursive subroutine calls, but that would
+  # require not only the N save-stack entries for the localized parameters, but
+  # N call frames too, so this approach saves both time and memory. It also
+  # avoids any risk of a "deep recursion" warning if you have a lot of
+  # parameters; and it avoids cluttering stack traces if the called method
+  # throws an exception. The downside is obviously that it uses "goto" for an
+  # otherwise-simple loop, but at least it's confined to this method. Further,
+  # the result is more than twice as fast as the recursive approach even when
+  # there are no parameters, and the "goto" version wins even more with larger
+  # parameter lists.
+  Loop:
+    return $self->$method(@args)
+      unless @param_names;
+    my $key = shift @param_names;
+    local $store->{$key}{params} = $params->{$key};
+    goto Loop; # Watch Dijkstra roll in his grave
 }
 
 sub _resolved_attrs { my $self = shift; $self->call_with_parameters($self->next::can, @_) }