call $callback via call_user_func

[tudborg]

I needed the ability to route to HTTPResource::GET, HTTPResource::PUT etc.
so i changed the way klein calls the route handlers.

By using call_user_func, any callable will be accepted as valid.

Should be fully backwards compatible.
Also added a test to ensure it actually works.

[chriso]

I'm not keen on merging this. From my experience call_user_func() has been slower than alternatives. If you can provide some benchmarks to show otherwise I'll merge. Also, there's no need to pass $request and $response by reference - objects are already passed by reference.

FYI, as a workaround you can make a string callable using the following pattern

<?php

function make_callable($function) {
    return function () use ($function) {
        return call_user_func_array($function, func_get_args());
    };
}

Here's an example

<?php
$phpinfo = make_callable('phpinfo');
$phpinfo();

[tudborg]

I'll create some benchmarks.
I also suspect it to be a tiny bit slower, but negligible. Let's see.

The passing by reference is needed when the callback is defined as a function that takes references. Otherwise call_user_func will fail since the function does not match the args given to call_user_func.
Alternatively, defining callbacks with reference parameters should be avoided?
I see no harm in declaring them as references, since it makes no difference except allow the use of callbacks declared with reference parameters.
(i will try and bench this too, just to be sure)

Your example is pretty much what i changed in the code, except it adds even more overhead to calling a static method.

Alternatively, you could send it through call_user_func only when it is actually needed.
Wrapped in function for ease of use

function make_callable($callback) {
   if(is_array($callback) || (is_string($callback) && strpos($callback, '::') !== false) ) {
      return function () use ($callback) {
         return call_user_func_array($callback, func_get_args());
      };
   }
   else {
      return $callback;
   }

}

But then again, if performance is an issue, that extra function call is a killer compared to what we are trying to avoid.

---

I made a bench of the above function vs. calling call_user_func plain.

Results:

1000000 iterations:
  with make_callable:
    class method:   4.7643320560455
    plain function: 0.97414493560791
    diff: 3.7901871204376

  without make_callable:
    class method:                 1.8997299671173
    plain through call_user_func: 0.50178503990173
    plain function:               0.15578007698059

  without make_callable with inline if:
    class method:   2.5600969791412
    plain function: 0.69795799255371
    diff: 2.4043169021606

(code https://gist.github.com/4326951 )

Obviously, sending it all trough a make_callable is a bad idea. We knew that.

Now, when looking at the middle result section, we see that using call_user_func with a plain function name, compared to calling it just by name has a diff time of 0.34600496292 seconds. devide that with the number of iterations and we get ~350 nanoseconds in average extra call time.

Let us assume that in any given app, a maximum number of 100 routes pr. request is called.

That will add 35ms extra time to the request.

Compared to any regex routing and whatever business logic might be in these fictive 100 routes, the 350 nanoseconds extra is nothing.
I think we could find those extra seconds somewhere else.

[tudborg]

(also notice that just calling it through call_user_func is cheaper than doing the if() with the is_string and strpos call)

[chriso]

Thanks for the analysis.

Not sure what you mean by "passing by reference is needed when the callback is defined as a function that takes references" - call-time pass by reference was deprecated in 5.3 and removed in 5.4.

[tudborg]

This is only an issue with call_user_func

$a = $b = $c = 10;

function f1($i, $j, $k) {
   return $i+$j+$k;
}
function f2(&$i, &$j, &$k) {
   return $i+$j+$k;
}

print call_user_func('f1', $a, $b, $c)."\n"; //ok
print call_user_func('f2', &$a, &$b, &$c)."\n"; //ok
print call_user_func('f2', $a, $b, $c)."\n"; //wont work

will output

30
30

Warning: Parameter 1 to f2() expected to be a reference, value given in /private/tmp/test.php on line 14

php version is 5.3.15

---

EDIT:

Also, it is not needed if everyone just defines route callbacks as seen in f1(),
and if klein should be php 5.4 compatible that is just the way to do it.

[chriso]

Interesting. I can't see why anyone would define a route handler like f2 since objects are already passed by reference - might as well aim for forward compat with 5.4. Happy to merge if you remove the references ;)

[Rican7]

I'm so lost at this point what this is even helping.
I've been running Klein on PHP 5.4.6-5.4.8 without any issues, and have even successfully called static OOP methods with the router.

See

1. https://github.com/Rican7/Paulus/blob/master/libs/Paulus/Router.php#L60
2. https://github.com/Rican7/Paulus/blob/master/routes/sample.php#L44

[tudborg]

i suspect this to be a feature of php5.4
In 5.3 you cannot call a static mehod by full string name.

Class C {
    static function f() {
        return "hello word";
    }
}

$f = 'C::f';
$f();

will output

Fatal error: Call to undefined function C::f() in /tmp/test.php on line 10

If you want to do this you would have to devide the class name and method name like so:

$c = 'C';
$f = 'f';
print $c::$f();

An alternate option for klein is to check for the '::' substring, or if callback is an array, and call in an alternate way if that is the case.

I will do a bench again to see if it is faster.

[tudborg]

Is it faster to just pass the string to call_user_func and let that handle it, or to check for :: and call it like $c::$f()
?

1000000 iterations:
checking for :: and is_array: 1.6051549911499
using call_user_func          0.54214596748352
diff:                         1.0630090236664

Obviously, using call_user_func is faster.
Another minor advantage of using call_user_func is that php decides what is callable, not klein. So whatever version of php you are running, that versions callables will be a valid.

( test code for good measure: https://gist.github.com/4335592 , and once again, php version 5.3.15 )

[tudborg]

there we go. References removed.

[chriso]

Thanks