Add optimizer support for prototype methods #7452

kocsismate · 2021-09-02T14:29:24Z

No description provided.

Zend/Optimizer/zend_func_info.c

TysonAndre · 2021-09-03T12:45:47Z

build/gen_stub.php

@@ -387,7 +391,7 @@ public function toOptimizerTypeMask(): string {
            case "iterable":
                return "MAY_BE_ARRAY|MAY_BE_ARRAY_KEY_ANY|MAY_BE_ARRAY_OF_ANY|MAY_BE_OBJECT";


I think MAY_BE_ARRAY_OF_REF would also make sense here?

TysonAndre · 2021-09-03T12:48:51Z

build/gen_stub.php

@@ -387,7 +391,7 @@ public function toOptimizerTypeMask(): string {
            case "iterable":


(GitHub doesn't allow commenting on the above line for case "callable":)

Does callable also need to include MAY_BE_ARRAY_OF_REF? If I use callable as a parameter type hint, I can still modify fields of an array callable by reference (and see them get modified)

php > class B { public function main() { echo "in main\n"; } } php > $class = new B(); php > $method = 'main'; php > $callable = [&$class, &$method]; php > $callable(); in main php > function example(callable $c) { $c(); $c[1] = 'changed'; } php > example($callable); in main php > var_dump($callable); array(2) { [0]=> &object(B)#1 (0) { } [1]=> &string(7) "changed" } php >

Not sure what the other implications of this would be for the optimizer, @nikic would know more (E.g. something with a callable type hint might easily cease to be a callable?)

TysonAndre · 2021-09-03T12:54:11Z

Zend/Optimizer/zend_func_info.c

-	zend_string *name = callee_func->common.function_name;
-	if (!name) {
+	zend_string *name = get_function_or_method_name(callee_func);
+	if (zend_string_equals_literal_ci(name, "main")) {


Isn't main already a valid name for a internal function or an instance method <?php function main() {}? "(main)" would at least avoid conflicts, though I'm not sure where else get_function_or_method_name is used

Is there a reason

Zend/Optimizer/zend_func_info.c

TimWolla · 2024-03-07T07:44:15Z

ext/random/random.stub.php

+        /** @return array<int, string> */
        public function pickArrayKeys(array $array, int $num): array {}


This is not correct, the returned array keys might be integers. The correct type would be list<int|string>.

iluuu1994 · 2024-03-08T12:13:06Z

Zend/Optimizer/zend_func_info.c

@@ -100,18 +101,20 @@ ZEND_API int zend_func_info_rid = -1;

 uint32_t zend_get_internal_func_info(
 		const zend_function *callee_func, const zend_call_info *call_info, const zend_ssa *ssa) {
-	if (callee_func->common.scope) {
-		/* This is a method, not a function. */
+	if (callee_func->common.scope && !call_info->is_prototype) {


Shouldn't this be callee_func->common.scope && call_info->is_prototype? is_prototype indicates we might be calling an overridden method.

iluuu1994 · 2024-03-08T12:16:53Z

Zend/Optimizer/zend_inference.c

@@ -3885,6 +3885,7 @@ static zend_always_inline zend_result _zend_update_type_info(
 		case ZEND_DO_ICALL:
 		case ZEND_DO_UCALL:
 		case ZEND_DO_FCALL_BY_NAME:
+		case ZEND_INIT_METHOD_CALL:


This looks wrong. ZEND_INIT_METHOD_CALL does not define a result. Methods are called using DO_FCALL, so this is already handled.

iluuu1994 · 2024-03-08T12:18:45Z

ext/opcache/tests/opt/method_func_info.phpt

+0002 T1 = FETCH_DIM_IS V0 int(0)
+0003 T0 = COALESCE T1 0005
+0004 T0 = QM_ASSIGN int(0)
+0005 VERIFY_RETURN_TYPE T0


Shouldn't these be optimized away then? What does this test test?

iluuu1994 · 2024-03-08T12:19:55Z

Zend/Optimizer/zend_optimizer.c

@@ -914,6 +914,28 @@ zend_function *zend_optimizer_get_called_func(
 					}
 					return fbc;
 				}
+			} else if (opline->op1_type == IS_CONST
+					   && Z_TYPE_P(CRT_CONSTANT(opline->op1)) == IS_OBJECT


Objects cannot be IS_CONST. I don't think this branch is hit. You should look at the inferred SSA variable instead.

Zend/Optimizer/zend_optimizer.c

iluuu1994 · 2024-03-08T12:23:02Z

Zend/Optimizer/zend_optimizer.c

+						/* Prototype methods are potentially overridden. fbc still contains useful type information.
+						 * Some optimizations may not be applied, like inlining or inferring the send-mode of superfluous args.
+						 * A method cannot be overridden if the class or method is final. */
+						if ((fbc->common.fn_flags & ZEND_ACC_FINAL) == 0 &&


Nit: && on next line.

iluuu1994 · 2024-03-08T18:01:30Z

Zend/Optimizer/zend_optimizer.c

+					   && !(op_array->scope->ce_flags & ZEND_ACC_TRAIT)) {
+				zend_string *method_name = Z_STR_P(CRT_CONSTANT(opline->op2) + 1);
+				zend_function *fbc = zend_hash_find_ptr(
+						&op_array->scope->function_table, method_name);


The class here should be based on op1 though, not op_array->scope.

kocsismate force-pushed the optimizer-method branch from f0ad035 to bdd4094 Compare September 2, 2021 14:40

nikic reviewed Sep 2, 2021

View reviewed changes

Zend/Optimizer/zend_func_info.c Outdated Show resolved Hide resolved

kocsismate force-pushed the optimizer-method branch 2 times, most recently from b390a1a to c7e5496 Compare September 2, 2021 15:22

kocsismate changed the title ~~Add optimizer support for methods~~ Add optimizer support for prototype methods Sep 2, 2021

kocsismate force-pushed the optimizer-method branch 2 times, most recently from c483aec to 610c6ad Compare September 2, 2021 16:47

TysonAndre reviewed Sep 3, 2021

View reviewed changes

kocsismate changed the base branch from master to PHP-8.1 September 5, 2021 18:53

kocsismate force-pushed the optimizer-method branch 2 times, most recently from c9f8b3b to 71e152d Compare September 6, 2021 15:21

kocsismate force-pushed the optimizer-method branch from 71e152d to ce8811e Compare October 17, 2021 09:55

Add optimizer support for methods

c91496b

kocsismate force-pushed the optimizer-method branch from ce8811e to c91496b Compare March 7, 2024 07:21

kocsismate changed the base branch from PHP-8.1 to master March 7, 2024 07:23

kocsismate requested review from dstogov, iluuu1994, TimWolla and zeriyoshi as code owners March 7, 2024 07:23

TimWolla reviewed Mar 7, 2024

View reviewed changes

iluuu1994 reviewed Mar 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add optimizer support for prototype methods #7452

Add optimizer support for prototype methods #7452

kocsismate commented Sep 2, 2021 •

edited

Loading

TysonAndre Sep 3, 2021

TysonAndre Sep 3, 2021

TysonAndre Sep 3, 2021

TimWolla Mar 7, 2024

iluuu1994 Mar 8, 2024

iluuu1994 Mar 8, 2024 •

edited

Loading

iluuu1994 Mar 8, 2024

iluuu1994 Mar 8, 2024

iluuu1994 Mar 8, 2024

iluuu1994 Mar 8, 2024 •

edited

Loading

		@@ -387,7 +391,7 @@ public function toOptimizerTypeMask(): string {
		case "iterable":
		return "MAY_BE_ARRAY\|MAY_BE_ARRAY_KEY_ANY\|MAY_BE_ARRAY_OF_ANY\|MAY_BE_OBJECT";

		/** @return array<int, string> */
		public function pickArrayKeys(array $array, int $num): array {}

Add optimizer support for prototype methods #7452

Are you sure you want to change the base?

Add optimizer support for prototype methods #7452

Conversation

kocsismate commented Sep 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iluuu1994 Mar 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iluuu1994 Mar 8, 2024 • edited Loading

Choose a reason for hiding this comment

kocsismate commented Sep 2, 2021 •

edited

Loading

iluuu1994 Mar 8, 2024 •

edited

Loading

iluuu1994 Mar 8, 2024 •

edited

Loading