Skip to content

Conversation

@kaja47
Copy link
Contributor

@kaja47 kaja47 commented Jan 29, 2026

  • Split variadic arguments for and() and or(). Common case of single argument doesn't allocate an array.
  • Add static fields mirroring $registry to skip one level of indirection.
  • Numeric values chosen that & and | is the same as min and max. and()/or() optimized accordingly.
  • Inline trivial create() method.
  • Remove one unnecessary access to registry in method create().

this loop runs 1.8x faster (php 8.4.16, jit off)

$t = TrinaryLogic::createYes();
for ($i = 10_000_000; $i--;) {
$t = $t->and(TrinaryLogic::createYes());
}

before: 1.125 s
now: 0.616 s

@kaja47 kaja47 marked this pull request as draft January 29, 2026 12:38
}

public function and(self ...$operands): self
public function and(self $operand, self ...$rest): self
Copy link
Contributor

@staabm staabm Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be
public function and(?self $operand = null, self ...$rest): self

because previously $operands could be a empty array (so no args given to and when invoked via splat
$trinary->and(...$moreTrinaries))

@staabm
Copy link
Contributor

staabm commented Jan 29, 2026

one bug in negate otherwise this looking really good already. awesome.


1) PHPStan\TrinaryLogicTest::testNegate with data set #0 (PHPStan\TrinaryLogic Object (...), PHPStan\TrinaryLogic Object (...))
Failed asserting that false is true.

/Users/m.staab/dvl/phpstan-src/tests/PHPStan/TrinaryLogicTest.php:105

2) PHPStan\TrinaryLogicTest::testNegate with data set #2 (PHPStan\TrinaryLogic Object (...), PHPStan\TrinaryLogic Object (...))
Failed asserting that false is true.

/Users/m.staab/dvl/phpstan-src/tests/PHPStan/TrinaryLogicTest.php:105

3) PHPStan\TrinaryLogicTest::testNegate with data set #1 (PHPStan\TrinaryLogic Object (...), PHPStan\TrinaryLogic Object (...))
Failed asserting that false is true.

/Users/m.staab/dvl/phpstan-src/tests/PHPStan/TrinaryLogicTest.php:105

@thg2k
Copy link
Contributor

thg2k commented Jan 29, 2026

Sorry if I miss something obvious, but what's the point of keeping the $registry static with the addition of the new $YES $NO $MAYBE? Isn't it fully redundant now?

- Split variadic arguments for and() and or(). Common case of single argument doesn't allocate an array.
- Add static fields mirroring $registry to skip one level of indirection.
- Numeric values chosen that & and | is the same as min and max. and()/or() optimized accordingly.
- Inline trivial create() method.
- Remove one unnecessary access to registry in method create().

this loop runs 1.8x faster (php 8.4.16, jit off)

$t = TrinaryLogic::createYes();
for ($i = 10_000_000; $i--;) {
	$t = $t->and(TrinaryLogic::createYes());
}

before: 1.125 s
now:    0.616 s
@ondrejmirtes
Copy link
Member

This is really nice! I welcome your low-level knowledge like this here 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants