Feature for Timer to measure callable performance

[rumpfc]

Description
This is a convinience feature enhancement for Timer to measure short code blocks without adding its start/stop methods before and after the block. The new method record() accepts a string and a callable as parameters.

// Example
$benchmark->record('slow_function', function () { slow_function('...'); });

Which is the equivalent to

$benchmark->start('slow_function');
slow_function('...');
$benchmark->stop('slow_function');

It can also return the callables return value

$length = $benchmark->record('string length', fn () => strlen('CI4'));
// $length = 3

Alternatively the common function timer() also accepts an optional callable for the same mechanics. Leaving out the 2nd parameter behaves like before (returning a Timer instance).

$length = timer('string length', fn () => strlen('CI4'));

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[MGatner]

Very nice PR! I like this new feature, especially the inclusion of callable which (in my opinion) allows the very valuable array notation benchmarking, not just closures. Our timer() function is starting to get a bit abstruse, but I'm okay with this expansion unless someone else objects.

Thanks for the contribution @rumpfc! A few notes inline, feel free to discuss.

[kenjis]

There are coding style violation. Please fix.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#php-style

[paulbalandan]

I have a question. If by mistake I made the callable throw an exception, then how can I get the elapsed time since $this->stop($name) will not be called? Should the invocation of the callable be wrapped in a try-finally ?

[rumpfc]

If an exception is thrown and you don't catch it, then I think you might have other problems than worrying about the time stoppage.

But you also have a point. It does not handle the case of purposely catching exceptions for error handling.

try {
    $benchmark->record('test', 'func_that_throws');
} catch (Throwable $e) {
    // stop() isn't called yet
}

@kenjis shall we cover this case as well?

[kenjis]

What if an exception is thrown after $benchmark->start() or in $iterator->run()?

[rumpfc]

For $iterator->run(), nothing is caught internally. It would not record anything in the report and would go out of the method, ignoring all follow up Closures.

For $benchmark->start(), you would need to call stop() inside the catch or finally block.

[kenjis]

I don't think we need try-finally.

An exception should be thrown in an exceptional case, and not recoverable.
Normally, a developer would modify the code to prevent the exception from occurring.

If a developer really wants to know the elapsed time when an exception is thrown, they can catch and measure it by themselves.

[kenjis]

Please fix coding style and failed tests.

[rumpfc]

Calling strlen without any parameters has 2 different behaviours between PHP7 and PHP8. That's why the unit test keep failing for 7.4, but not for 8.0 and 8.1:

// PHP 7.4 and before
strlen();   // throws ErrorException (extends Exception)

// PHP 8.0 and after
strlen();   // throws ArgumentCountError (extends TypeError)

@kenjis I can offer 2 solutions:

1. Unit test expects a Throwable to be thrown
2. I catch ErrorException inside record() and throw an ArgumentCountError

[kenjis]

@rumpfc Not specifically catch Exception in the method. So it's weird to catch only ErrorException.
I don't know all ErrorExceptions in PHP7 will be ArgumentCountError in PHP8.

So it's okay to expect Throwable in the test.
Also, please comment why we need to use Throwable.

[kenjis]

LGTM!

[MGatner]

Thanks all!