Prevent unnecessary isSuperTypeOf() calls

[staabm]

No description provided.

[ondrejmirtes]

Thank you.

[thg2k]

Does this change bring any measurable benefit? Looks like it just reduces the readability.

[staabm]

I don't have a concrete repro case for the change.

My experience from watching at tens of profiles over the past is, that phpstan is CPU bound, mostly on Type method calls, especially isSuperTypeOf() which can have very different characteristics if e.g. big array types are involved.

[mad-briller]

I've seen this pattern elsewhere in *Type classes too and considered this change but never sent a pr as i couldn't prove any benefits, because it is pretty hard to create repro cases for and the readability does suffer.

One thing i have investigated in the past is a compromise between the two using generators:

// IterableType.php

return $limit->and(
	$otherType->isIterable(),
	$otherType->getIterableValueType()->isSuperTypeOf($this->itemType),
	$otherType->getIterableKeyType()->isSuperTypeOf($this->keyType),
);

could become:

return $limit->yieldAnd(function() use ($otherType): Generator {
	yield $otherType->isIterable();
	yield $otherType->getIterableValueType()->isSuperTypeOf($this->itemType);
	yield $otherType->getIterableKeyType()->isSuperTypeOf($this->keyType);
});

This generator approach would allow complex logic to be done in a lazy way, including loops, conditionals etc. without suffering too much readability loss; but again because i could never come up with concrete gains so i never sent a pr.

[ondrejmirtes]

There's already TrinaryLogic::lazyAnd() for that.

[mad-briller]

yeah that was another reason i didn't send it 'cos having a third option would cause confusion aswell, however i have run into some limitations around lazyAnd that makes it awkward to use in some cases.

Not all cases you want to be lazy involve applying the same closure to a list of objects, for example how could you refactor the given example from IterableType to use lazyAnd()? i'm not sure you could

[ondrejmirtes]

Yeah, you would save the isSuperTypeOf, that would be lazy, but you would not save the $otherType->isIterable() etc. calls first.

[thg2k]

I'm in favor of cpu optimizations when they make sense, but here it really looks like we are gaining pennies and paying more in readability. A wise person once told me: Programmers time is more valuable than CPU time.

[ondrejmirtes]

Programmers time is more valuable than CPU time

Yes, this holds true. That's why it's always worth it to make PHPStan faster, because it saves times for programmers all around the planet. (Not just PHPStan programmers, but a lot of other PHP programmers as well.)

[ondrejmirtes]

Locking this - don't take this personally, but people always love to bikeshed the smallest of issues, instead of helping where the help would matter :)