Is the cost of php 8 Attributes worth it?

[bulletproof-coding]

I see more and more people want to use them when other faster solutions exist. Example: #52004

In my opinion they should be used only when they are the only option available, the big con being that they need extra code to be executed, resulting in slower execution speed, higher processing load and if not used, in useless overhead.

[bulletproof-coding]

If they will /are implemented, can they be implemented so that no extra code gets executed if someone does not use them?

Like for example from that pull request, without this being executed:

$reflection = new ReflectionClass($command);

if ($attribute = $reflection->getAttributes(OnQueue::class)[0] ?? null) {

[mbabker]

Making sure I'm understanding correctly. Because of your concerns with the performance of processing attributes (if this is indeed measurable, please provide benchmarks confirming your findings so that others are able to make informed decisions regarding this), your suggestion is to introduce extra code to be executed, resulting in slower execution speed and higher processing load, to ensure attributes handling code is disabled. Correct?

[bulletproof-coding]

@mbabker I mean can it be implemented in laravel like:

if (\config('attributes.enabled')) { 
    $reflection = new ReflectionClass($command);
    $attributes = $reflection->getAttributes(OnQueue::class);

    if ($attributes !== []) {
        // code
    }
}

So we can enable or disable them from config (via env).

"your suggestion is to introduce extra code to be executed, resulting in slower execution speed and higher processing load, to ensure attributes handling code is disabled. Correct?" - NO.

....

[mbabker]

How is the introduction of a runtime Illuminate\Contracts\Config\Repository::get() call made from an indeterminate number of locations in the framework whose default implementation will reach into Illuminate\Support\Arr::get() an indeterminate number of times going to be more performant than just invoking (new \Reflector($thing))->getAttributes();, or again using your words, not introducing extra code to be executed that results in slower execution speed"?

You are making the claim that there is a hugely negative performance tradeoff in the use of attributes, I would expect this claim to be backed up with data to help people make an informed decision on their use instead of forcing people to make an emotional decision based on one person's claims, especially when it comes to justifying the suggestion that said performance issue can be negated by introducing more runtime code to avoid triggering the potentially performance invasive code.

[bulletproof-coding]

@mbabker I can agree with you that this introduces overhead by calling that config, hence my question:

"If they will /are implemented, can they be implemented so that no extra code gets executed if someone does not use them?"

Also found this interesting article about adnotations: https://r.je/php-annotations-are-an-abomination

PS. I don't want to get into politics with how much they will slow down the executions. The key point is that they will. If someone decides it is worth it, then so be it.

[mbabker]

Depending on the implementation, if something is adding attribute support alongside another way of configuring something, it would be possible to avoid triggering Reflection with guard clauses and early returns. But if you're looking for an implementation like you've proposed with the config, then you're introducing a performance hit for pulling the config repo out of the container and reading the array that in terms of micro optimizations probably isn't worth it over just running Reflection.

Yeah, in general, going to Reflection has a less optimal performance over other stuff like the framework already supports and uses, but unless we're talking about hot path code where those milliseconds matter, I'm not convinced a global killswitch with its own performance concerns (or the introduction of even more global static state in the framework) is worth the effort.