[9.x] Improve Performance On Terminate

[SupianIDz]

count is used in a loop and is a low performance construction

based on several experiments, I conclude that using foreach is better. Indeed, this performance increase does not really affect the framework, but why not?

• Tested on Ubuntu Server 20.04 (dual core cpu, 4 gb ram, first image)
• Tested on MacBook Air M1 2020 (second image)
• First order is better
• The test is carried out 10 times, each time the test calls 100 callbacks

Ubuntu Server

MacBook Air

<?php

// create 100 dummy callback
$data = array_map(static function () {
    return static function () {
        // something
    };
}, range(1, 100));

function bench(array $data)
{
    $results = [];

// while
    $start = microtime(true);

    $i = 0;
    while ($i < count($data)) {
        $data[$i]();
        $i++;
    }

    $results['while'] = microtime(true) - $start;

// for
    $start = microtime(true);

    for ($i = 0; $i < count($data); $i++) {
        $data[$i]();
    }

    $results['for'] = microtime(true) - $start;

// foreach
    $start = microtime(true);

    foreach ($data as $value) {
        $value();
    }

    $results['foreach'] = microtime(true) - $start;

// print result

    asort($results);

    foreach ($results as $key => $value) {
        echo $key . ': ' . $value . PHP_EOL;
    }
}

// run

for ($i = 0; $i <= 10; $i++) {
    bench($data);

    echo PHP_EOL;
}

[eusonlito]

But foreach use a in-memory copy of terminatingCallbacks array, and I think that this array can be edited on the fly adding more elements while is being executed. Then, the new elements will not be executed using foreach.

[GrahamCampbell]

I think this is possibly a breaking change? People could have been messing with what was in that array as part of a termination handler. 10.x is probably the safest place for this, especially since the performance improvement is only very minor.

[SupianIDz]

I think this is possibly a breaking change? People could have been messing with what was in that array as part of a termination handler. 10.x is probably the safest place for this, especially since the performance improvement is only very minor.

I don't think it would be a breaking change, the function to register the termination callback uses an integer index on the array, and based on the previous code uses an integer index to call.

See :

framework/src/Illuminate/Foundation/Application.php

Lines 1149 to 1176 in cf29656

	/**
	* Register a terminating callback with the application.
	*
	* @param callable|string $callback
	* @return $this
	*/
	public function terminating($callback)
	{
	$this->terminatingCallbacks[] = $callback;
	return $this;
	}
	/**
	* Terminate the application.
	*
	* @return void
	*/
	public function terminate()
	{
	$index = 0;
	while ($index < count($this->terminatingCallbacks)) {
	$this->call($this->terminatingCallbacks[$index]);
	$index++;
	}
	}

[X-Coder264]

@SupianIDz This was changed on purpose to the way it is currently and the performance impact was well explained in the MR -> #39175

And yes, reverting that change would be a breaking change.

[netpok]

Maybe a test would be great to verify that behavior

[taylorotwell]

No plans to change this.