inline calls in TrinaryLogic to reduce method call overhead

[staabm]

looking at perf profiles of the slow test https://github.com/phpstan/phpstan-src/commit/d33454d17333bc953d7399e8f6851f237a0bb37d we can see TrinaryLogic creation is a major cost.

with this PR I propose to "duplicate" a few lines of code to reduce the callstack in this very perf critical path

with blackfire I can see a 30% perf improvement before/after this PR when running

blackfire run php vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php --filter testBug5081

refs https://github.com/phpstan/phpstan/issues/7666#issuecomment-1195176312

the perf improvement is also visible when running plain

time vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php

before this PR 15-17secs; after 13-15 secs

env

php -v
PHP 8.1.2 (cli) (built: Jul 21 2022 12:10:37) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.2, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.2, Copyright (c), by Zend Technologies
    with blackfire v1.80.0~linux-x64-non_zts81, https://blackfire.io, by Blackfire

[herndlm]

This has such an impact although it was already implemented in create?

[staabm]

I think the impact is the actual call-overhead to create() not the logic implemented in the method. we call it milions of times and that adds up.

[oprypkhantc]

Just curious, can't PHPStan use native enums? That would likely reduce the overhead even more, although would require bumping up PHP requirement to 8.1.

[canvural]

Just curious, can't PHPStan use native enums? That would likely reduce the overhead even more, although would require bumping up PHP requirement to 8.1.

PHPStan's source code is downgraded with Rector in the PHAR build. And I guess Rector can't downgrade native enums. That's why.

[staabm]

after this change, the TrinaryLogic is no longer on the slow path. IMO its fine as is.

if we find a case where its still measurably slow, we can think about enums IMO.

this PR is about reducing function/method call overhead. using enums would only help us in this regard if we would directly access the enum cases instead of invoking a method like we do atm with TrinaryLogic::createYes() etc

its only relevant for super huge union types which are rare.

[ondrejmirtes]

I was tempted to merge this, but this looks for the problem in the wrong place.

Sure, if you call TrinaryLogic a billion times, optimization like that can help.

The problem is that the previous version didn't call TrinaryLogic a billion times, so this problem didn't exist.

We need to fix the root cause, and not treat the symptoms.

More about my findings here: https://github.com/phpstan/phpstan/issues/7666#issuecomment-1200290771

[staabm]

I agree this one is not the fix for the root cause.

Still its a easy perf improvement for one of the most used primitives in phpstan, without a relevant cost/complexity increase.

IMO it should be considered for merge as it will improve performance for cases with big/huge union types

[ondrejmirtes]

Yeah, I don't see why not. Thanks!