Item9345: Consider using pass-by-reference in VirtualHostingContrib

Priority:	CurrentState:	AppliesTo:	Component:	WaitingFor:
Enhancement	Closed	Extension	VirtualHostingContrib	Main.AntonioTerceiro, Main.JoenioCosta

Consider using pass-by-reference at Foswiki::Contrib::VirtualHostingContrib::VirtualHost::_run_with_local_configuration()

In first place: Congratulations, AntonioTerceiro and JoenioCosta! This is a great extension!

I read the code and I think the _run_with_local_configuration() can be enhanced to use less memory (and to be a little faster) if you pass the config hash in a reference, instead of it's value. The first call would look like this:

$self->_run_with_local_configuration($code, { %{$self->{config}} });

And then (untested):

84    sub _run_with_local_configuration {
85     my ($self, $code, $config) = @_;
86     if (scalar(%$config)) {
87       my $key = (keys(%$config))[0];
88
89       local $Foswiki::cfg{$key} = _merge_config($Foswiki::cfg{$key}, $config->{$key});
90       delete $config->{$key};
91       $self->_run_with_local_configuration($code, $config);
92     } else {
93       &$code();
94     }
95   }

This way a copy of %config is done in the first time the method is called and the copy is consumed as the recursive calls are made. Finally the copy is fully consumed, turning into an empty hash. Besides, passing a single reference is faster than a whole list.

In the way it's implemented, many copies of %config (or subsets of it) accumulate on the stack...

-- GilmarSantosJr - 16 Jul 2010

sounds good, we'll try it in the next weeks (busy times)

-- AntonioTerceiro - 19 Jul 2010

Thanks for the extension, though, is there any guidance on how to modify the apache httpd.conf files? I am having trouble with "Foswiki has received a suspicious change request from your browser. "

-- JoelDeJesus - 04 Aug 2010

this was changed and released in release 0.2.0 of VirtualHostingContrib

-- AntonioTerceiro - 02 Nov 2010

Then why not go all the way and localise them all using a hashslice to localise the keys:

local @Foswiki::cfg{keys %{$self->{config}}};

Then you can safely loop over the keys and merge their values, and finally call the code. Haven't tested, so it might be:

local @{$Foswiki::cfg}{keys %{$self->{config}}};

-- OlivierRaginel - 02 Nov 2010

this seems to work (all unit tests pass):

Index: VirtualHost.pm
===================================================================
--- VirtualHost.pm  (revisão 9824)
+++ VirtualHost.pm  (cópia de trabalho)
@@ -74,26 +74,9 @@

   local $Foswiki::Contrib::VirtualHostingContrib::VirtualHost::CURRENT = $self->hostname;

-  $self->_run_with_local_configuration($code, { %{$self->{config}} });
-}
+  local @Foswiki::cfg{keys %{$self->{config}}} = map { _merge_config($Foswiki::cfg{$_}, $self->{config}->{$_}) } (keys %{$self->{config}});

-# Recursive method; consumes the local configuration, updates Foswiki global
-# configuration with local() and runs the code inside the virtualhost.  This
-# method needs to be recursive because if the configurations were set in a
-# loop, the local() declarations would lose their scope just after the loop.
-#
-# Note that $config will be consumed in recursive calls, so make sure you pass
-# a copy of your actual data.
-sub _run_with_local_configuration {
-  my ($self, $code, $config) = @_;
-  if (scalar(%$config)) {
-    my $key = (keys(%$config))[0];
-    local $Foswiki::cfg{$key} = _merge_config($Foswiki::cfg{$key}, $config->{$key});
-    delete $config->{$key};
-    $self->_run_with_local_configuration($code, $config);
-  } else {
-    &$code();
-  }
+  &$code();
 }

 sub _merge_config {

-- AntonioTerceiro - 02 Nov 2010

released a new package with this change (0.3.0)

-- AntonioTerceiro - 02 Nov 2010