[Xapian-discuss] Another PHP 5 wrapper...

Tue Apr 18 18:33:18 BST 2006

[Olly, Paul, Sam : please forgive me for being absent, I was not able to 
follow this thread as i wished...]

Olly Betts a écrit :

>And here's the wrapper file it produces for Xapian
>http://www.oligarchy.co.uk/xapian/patches/xapian.phps
>  
>
This is great news!
I looked at the generated wrapper and made some quick tests : it looks 
very nice, and it works !

You worked so fast that I'm going to ask more ;-)

>I've used Paul's ideas for coercing parameters and return values, except
>that I'm generating wrapper methods so I can skip doing that when I know
>it won't be needed (currently the check errs on the side of coercing
>when it sometimes isn't needed which doesn't affect correctness, but
>just adds a small overhead).
>  
>
It is probably a good thing to generate wrapper methods and not using 
the "__call" magic function of php, because it seems to add a 
substantial overhead.

I made some quick performance tests (just repeatedly adding some data 
and postings to a new document), comparing native calls to the xapian 
library with the use of the SWIG's generated wrapper, Paul's one and mine.

Test code is here :
http://www.bdsp.tm.fr/aed/xapian/call_test.php.txt

On my machine, results are as follows :
- using native calls : 8,88 seconds
- using my own wrapper : 13,88 sec
- using Paul's wrapper : 64,12 sec
- using the wrapper generated by SWIG : 37,54 sec.

I think that the difference between the 3 wrappers is only due to the 
use of __call, the coercion of parameters, and the use of 
call_user_func_array.

SWIG's wrapper don't use __call, but it seems that using 
func_get_args/array_walk_recursive/array_unshift/call_user_func_array 
for each call still takes time.

Sam, in a previous mail, suggested that it would be possible to do the 
coercion of params directly in the generated C code, but I was wondering 
if it would be possible to do most of the work during the generation.

I saw in your patch that we have access to all the parameters of the 
function we wrap. Perhaps it would be possible to write all the 
parameters of the generated PHP method and then do a direct (real) call 
to the flat Xapian function, coercizing (?) on the fly all the 
parameters for which it is necessary. (I'm not a "C" person, and I don't 
know SWIG at all, so forgive me if it is just stupid !)

So instead of having (line 1045 of patched php4.cxx)
    Printf(s_oowrappers, "\tpublic function %s() {\n", methodname);

we would have something like :
    Printf(s_oowrappers, "\tpublic function %s(", methodname);
    p=l;  // l is ParmList *l = Getattr(n,"parms");
    while(p)
    {
        if (p!=l) Printf(s_oowrappers, ", ");
        Printf(s_oowrappers, p->name);
        // Printf(s_oowrappers, "%s %s", p->type, p->name); // php5 
only, add type hints for parameters
        p=nextSibling(p);
    }
    Printf(s_oowrappers, ") {\n");

and then, instead of having (lines 1062 and others)
    Printf(s_oowrappers, "\t\t$a=func_get_args();\n");
    Printf(s_oowrappers, 
"\t\tarray_walk_recursive($a,'swig_coerce_param');\n");
    Printf(s_oowrappers, "\t\tarray_unshift($a,$this->swig_ptr);\n");
    Printf(invoke, "call_user_func_array('%s',$a)", iname);

we would have another loop doing something similar to :
    Printf(invoke, "%s($this->swig_ptr", iname);
    p=l;
    while(p)
    {
        Printf(invoke, ", ");
        if (p->type == T_USER)
        {
            Printf(invoke,"%s->swig_ptr", p->name);
        }
        else
        {
            Printf(invoke, p->name);
        }
        p=nextSibling(p);
    }
    Printf(invoke, ")");

I am pretty sure that it would be more complicated than that (handling 
parameters default values, for example), but there would be some benefits :
- generated code would be cleaner (just direct calls to the Xapian 
functions)
- probably the same speed as my original wrapper
- methods arguments are "documented" (I mean : they appear in the method 
signature), so one don't have to look at the doc to guess the parms, and 
it allows auto-completion in IDEs

>(in a response to Sam about swig_coerce_param) It's also nice that we can 
>automatically do it recursively in PHP - this means that the 
>typemap for constructing a Query from a PHP array of Query 
>objects just works without having to rewrite it.
>  
>
My pseudo solution above won't handle that, except if we add a special 
case for that (if arg is an array, walk the array to detect things to 
coerce and create a new array). But do we really need it ? only 
new_query uses this (?), and it seems to me that it is already very 
flexible...

>I'm not renaming methods to lowerCamelCase yet (I've written and
>submitted a patch to add support for this to SWIG so it's easy enough to
>do once that gets applied).
>  
>
This is another good news !
Perhaps that it should be a swig option (on : camelize method names, off 
: use method names "as-is")

>I could easily wrap non-member functions (...).
>
>Daniel wrapped the functions and constants in a Xapian class in his OO
>bindings, and it does means everything we wrap is in a pseudo namespace,
>which is cleaner.
>  
>
I won't be against this solution ;-), but here is two (less clean in my 
opinion) alternatives:
- add the static methods to the XapianSwigBase class (they will be 
available in all classes... more confusing than usefull)
- if we use camelCase, functions names don't conflict anymore with flat 
names

>SWIG's Python backend allows you to generate pydoc
>strings (either automatically, or specified explicitly, or a mix of
>the two).  There's no good reason why the PHP backend couldn't too.
>  
>
It would be fantastic to have the same thing for PHP, but does it mean 
that you have to "duplicate" the doc in xapian.i or something like that 
? I guess it can raise a maintenance issue (but I had the same problem 
in my wrapper).

>(in a previous mail)
>Is there a way we can write code which uses Countable for PHP 5.1 and
>later but not on 5.0?
>  
>
I think that we can all live with a wrapper without the "Countable" 
interface...
Using it and supporting both php 5.0 and php 5.1 would require to add 
something like this in the wrapper :
    if (! interface_exists('Countable'))
    {
        interface Countable{};
    }

But PHP function 'interface_exists' was only added in php 5.0.2, so it 
is not a perfect solution.

Some additional remarks about the current wrapper :

I think we should avoid, if we can, having global functions or constants 
in the generated wrapper (avoiding "global namespace pollution") :
- I am not sure that the "$XAPIAN_LOADED__" stuff at the top of the file 
is really necessary (php has function like include_once or 
require_once), but perhaps it is automatically generated by SWIG ?
- in Paul's wrapper, functions like swig_coerce_param and 
swig_wrap_return were members of the base class. I think it is a better 
solution than putting them global.

The PHP function "is_a" (used in coerce_param) is deprecated in PHP5 ans 
is replace by "instanceof".
    if ($value instanceof XapianSwigBase) $value=$value->swig_ptr;

Please note that, except that, your generated wrapper doesn't have any 
warning, even in E_STRICT mode.

You really did a great job !

Oh, by the way, congratulations for the 0.9.5 release ! I didn't have 
time to test, but I will, believe me ;-)

Best regards,

-- 

Daniel Ménard

Banque de Données Santé Publique
Avenue du Professeur Léon Bernard
35043 Rennes Cédex

Tél. (+33) 2.99.02.29.42
Fax (+33) 2.99.02.26.28
E-mail Daniel.Menard at Bdsp.tm.fr
http://www.bdsp.tm.fr