Skip to content

Add tests for function/class declared in a file included repeatedly #7563

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Zend/tests/repeated_include/anonymous_class.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

$className = AnonymousClassNameCache::get_class(fn () => new class() {
public static function verify() {
return 'ok';
}
});
64 changes: 64 additions & 0 deletions Zend/tests/repeated_include/anonymous_class.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
--TEST--
Repeated include must not increase memory - anonymous class
--SKIPIF--
<?php
// TODO test should pass even without opcache, but fixes are needed
// related bug tracker https://bugs.php.net/bug.php?id=76982
if (!extension_loaded('Zend OPcache') || !(opcache_get_status() ?: ['opcache_enabled' => false])['opcache_enabled']) {
die('xfail test currently requires opcache enabled');
}
?>
--FILE--
<?php

// new anonymous class declared and included multiple times always has a new class name,
// this test accounts for that and tests if an anonymous class declared in a closure
// invoked once does not increase memory consumption

final class AnonymousClassNameCache
{
/** @var array<string, string> */
private static $classNameByFxHash = [];

private function __construct()
{
}

public static function get_class(\Closure $createAnonymousClassFx): string
{
$fxRefl = new \ReflectionFunction($createAnonymousClassFx);
$fxHash = $fxRefl->getFileName() . ':' . $fxRefl->getStartLine() . '-' . $fxRefl->getEndLine();

if (!isset(self::$classNameByFxHash[$fxHash])) {
self::$classNameByFxHash[$fxHash] = get_class($createAnonymousClassFx());
}

return self::$classNameByFxHash[$fxHash];
}
}

$classNamePrev = null;
$m = -1;
$mDiff = -1;
$mPrev = 0;
for ($i = 0; $i < (PHP_OS_FAMILY === 'Windows' ? 1_000 /* include is slow on Windows */ : 20_000); $i++) {
Copy link
Member

Choose a reason for hiding this comment

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

If 1,000 iterations are enough for Windows, why use 20,000 on others?

Copy link
Contributor Author

@mvorisek mvorisek Apr 20, 2023

Choose a reason for hiding this comment

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

On some systems I needed several hundreds iterations to trigger a memory increase. So I figured 20k is good count to test this extensively and keep it still fast (<1 second), in Windows, the repeated include is quite costly, so on Windows, only 1k iterations are tested in hope it will be enough and to keep the test/CI still fast.

require __DIR__ . '/anonymous_class.inc';
if ($classNamePrev !== null && $className !== $classNamePrev) {
echo 'Class name is different: ' . $className . ' vs ' . $classNamePrev . ' (' . $i . ' iteration)' . "\n";
exit(1);
}
$classNamePrev = $className;
assert($className::verify() === 'ok');
$m = memory_get_usage();
$mDiff = $m - $mPrev;
if ($mPrev !== 0 && $mDiff !== 0) {
echo 'Increased memory detected: ' . $mDiff . ' B (' . $i . ' iteration)' . "\n";
exit(1);
}
$mPrev = $m;
}
echo 'done';

?>
--EXPECT--
done
7 changes: 7 additions & 0 deletions Zend/tests/repeated_include/anonymous_function.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

$funcA = function () {
return 'ok';
};

$funcB = fn () => 'okB';
33 changes: 33 additions & 0 deletions Zend/tests/repeated_include/anonymous_function.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
--TEST--
Repeated include must not increase memory - anonymous function
--SKIPIF--
<?php
// TODO test should pass even without opcache, but fixes are needed
// related bug tracker https://bugs.php.net/bug.php?id=76982
if (!extension_loaded('Zend OPcache') || !(opcache_get_status() ?: ['opcache_enabled' => false])['opcache_enabled']) {
die('xfail test currently requires opcache enabled');
}
?>
--FILE--
<?php

$m = -1;
$mDiff = -1;
$mPrev = 0;
for ($i = 0; $i < (PHP_OS_FAMILY === 'Windows' ? 1_000 /* include is slow on Windows */ : 20_000); $i++) {
require __DIR__ . '/anonymous_function.inc';
assert($funcA() === 'ok');
assert($funcB() === 'okB');
$m = memory_get_usage();
$mDiff = $m - $mPrev;
if ($mPrev !== 0 && $mDiff !== 0) {
echo 'Increased memory detected: ' . $mDiff . ' B (' . $i . ' iteration)' . "\n";
exit(1);
}
$mPrev = $m;
}
echo 'done';

?>
--EXPECT--
done
9 changes: 9 additions & 0 deletions Zend/tests/repeated_include/class_in_if.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

if (!class_exists(Test::class)) {
class Test {
public static function verify() {
return 'ok';
}
}
}
32 changes: 32 additions & 0 deletions Zend/tests/repeated_include/class_in_if.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--TEST--
Repeated include must not increase memory - class in if condition
--SKIPIF--
<?php
// TODO test should pass even without opcache, but fixes are needed
// related bug tracker https://bugs.php.net/bug.php?id=76982
if (!extension_loaded('Zend OPcache') || !(opcache_get_status() ?: ['opcache_enabled' => false])['opcache_enabled']) {
die('xfail test currently requires opcache enabled');
}
?>
--FILE--
<?php

$m = -1;
$mDiff = -1;
$mPrev = 0;
for ($i = 0; $i < (PHP_OS_FAMILY === 'Windows' ? 1_000 /* include is slow on Windows */ : 20_000); $i++) {
require __DIR__ . '/class_in_if.inc';
assert(Test::verify() === 'ok');
$m = memory_get_usage();
$mDiff = $m - $mPrev;
if ($mPrev !== 0 && $mDiff !== 0) {
echo 'Increased memory detected: ' . $mDiff . ' B (' . $i . ' iteration)' . "\n";
exit(1);
}
$mPrev = $m;
}
echo 'done';

?>
--EXPECT--
done
7 changes: 7 additions & 0 deletions Zend/tests/repeated_include/function_in_if.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

if (!function_exists('test')) {
function test() {
return 'ok';
}
}
32 changes: 32 additions & 0 deletions Zend/tests/repeated_include/function_in_if.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--TEST--
Repeated include must not increase memory - function in if condition
--SKIPIF--
<?php
// TODO test should pass even without opcache, but fixes are needed
// related bug tracker https://bugs.php.net/bug.php?id=76982
if (!extension_loaded('Zend OPcache') || !(opcache_get_status() ?: ['opcache_enabled' => false])['opcache_enabled']) {
die('xfail test currently requires opcache enabled');
}
?>
--FILE--
<?php

$m = -1;
$mDiff = -1;
$mPrev = 0;
for ($i = 0; $i < (PHP_OS_FAMILY === 'Windows' ? 1_000 /* include is slow on Windows */ : 20_000); $i++) {
require __DIR__ . '/function_in_if.inc';
assert(test() === 'ok');
$m = memory_get_usage();
$mDiff = $m - $mPrev;
if ($mPrev !== 0 && $mDiff !== 0) {
echo 'Increased memory detected: ' . $mDiff . ' B (' . $i . ' iteration)' . "\n";
exit(1);
}
$mPrev = $m;
}
echo 'done';

?>
--EXPECT--
done