[RFC] Internal operator overloading and GMP improvements View as Markdown

Hi Nikita,

Thanks for function renaming.
I'm agree about INC/DEC and unary operators (!, ~) implementation.

A single callback for all operators may not be always good, because classes
may overload not all but only few operators,
but technically it doesn't make any problems, so let keep it as is.

According to fast-path execution, I would really like to not introduce the
additional checks.
Especially for concat_function I would change it in the following way:

 if (Z_TYPE_P(op1) != IS_STRING) {

if (Z_TYPE_P(op1) == IS_OBJECT && Z_OBJ_HANDLER_P(op1, do_operation))
{
return Z_OBJ_HANDLER_P(op1, do_operation)(ZEND_CONCAT, result, op1,
op2 TSRMLS_CC);
}
zend_make_printable_zval(op1, &op1_copy, &use_copy1);
}
if (Z_TYPE_P(op2) != IS_STRING) {
if (Z_TYPE_P(op2) == IS_OBJECT && Z_OBJ_HANDLER_P(op2, do_operation))
{
return Z_OBJ_HANDLER_P(op2, do_operation)(ZEND_CONCAT, result, op1,
op2 TSRMLS_CC);
} zend_make_printable_zval(op2, &op2_copy, &use_copy2);
}

And in similar way for mod, shift, etc

if (UNEXPECTED(Z_TYPE_P(op1) != IS_LONG)) {

if (Z_TYPE_P(op1) == IS_OBJECT && Z_OBJ_HANDLER_P(op1, do_operation))
{
return Z_OBJ_HANDLER_P(op1, do_operation)(ZEND_MOD, result, op1, op2
TSRMLS_CC);
}
zendi_convert_to_long(op1, op1_copy, result);
}
op1_lval = Z_LVAL_P(op1);
if (UNEXPECTED(Z_TYPE_P(op2) != IS_LONG)) {

if (Z_TYPE_P(op2) == IS_OBJECT && Z_OBJ_HANDLER_P(op2, do_operation))
{
return Z_OBJ_HANDLER_P(op2, do_operation)(ZEND_MOD, result, op1, op2
TSRMLS_CC);
} zendi_convert_to_long(op2, op2_copy, result);
}

I think this is the last technical issue :)

Thanks. Dmitry.

Hi Nikita,

Few final notes:

• I wouldn't change zend_object_compare_t into
  zend_object_compare_objects_t. It would be better to name the new function
  as zend_object_compare_zvals_t. (It's just for better backward
  compatibility)

Done. Should I also call the handler itself compare_zvals then or can that
stay as just compare?

• Increment and decrement operators in PHP may have different semantic

than +=1, but I it's probably OK to use ADD/SUB for them.

I think we should just introduce this once a use case comes up.

• In some cases you insert call to zend_object_do_operation into the most
  probable path (e.g. in mod_function). This would cause at lease 2
  additional comparisons and may be conditional jumps. I think it would be
  better to check for most probable operand types first...

For $a % $b the most common cases are already handled by
fast_mod_function. mod_function is only directly called when doing $a %=
$b. But in any case, I'm not sure how I could test the more probable cases
first, as the next thing the code does is call convert_to_long. Of course I
could copy the code from fast_mod_function in there, but that doesn't
sounds like a good idea.

I think the only operation that could really be a performance concern is
concat_function, because that's a common operation without a fast_ variant.
But even in that case I could not measure a difference in runtime (taking
averages on 100M concatenations).

Also it may be better to use a table of callbacks for each overloaded
operand instead of single one that need to do switch anyway.

What would be the benefits? Better performance? Imho using a switch is
handier when implementing the operators, because it requires less
boilerplate code (no need to repeat function signature, variables, shared
code etc).

Also it may make sense to think about overloading of unary operators to
provide a solid decision.

You mean being able to overload unary + and - directly rather than the
current 0+$a / 0-$a transformation? I can see that this might be useful,
but not sure it's worth it (would have to introduce new opcodes for that).
I'd do the same as with the increment/decrement operators here: Only
implement them once there is a specific use case.

In case you think about user-level operator overloading in the future

(that may make sense :) it would be better to design them all together.

I was thinking about that too, but from the previous discussions on the
topic I figured that there is zero chance for having that in PHP :/

Thanks for the feedback!
Nikita