Do not use proxy-decorators and event listeners when debug is skipped

[xepozz]

There are a few checks that disable data collecting:

• yii-debug/src/Debugger.php

  Line 43 in cd4fe68

  $this->skipCollect = true;

• yii-debug/src/Debugger.php

  Line 48 in cd4fe68

  $this->skipCollect = true;

But they do not unwrap proxied classes, like \Yiisoft\Yii\Debug\Collector\LoggerInterfaceProxy or any other.
Also they do not unsubscribe from event-dispatcher.

It may negatively impact performance.

An idea to control if data collecting is skipped:

• Create a proxy class for *InterfaceProxy classes
• All *InterfaceProxy must expose decorated class
• If data collecting is skipped, call $proxy->getDecorated()->$method($args) directly
• Otherwise call $proxy->$method($args)

Or add check is data collecting is skipped to each public method of proxy classes

[samdark]

👍 That would definitely be great for performance.

Also they do not unsubscribe from event-dispatcher.

I'd prevent subscribing in this case if possible.

[samdark]

Checked current implementation. Am I right in my research, @xepozz? What are your thoughts?

Events

Currently event subscription is done via configs:

• https://github.com/yiisoft/yii-debug/blob/master/config/events-web.php
• https://github.com/yiisoft/yii-debug/blob/master/config/events-console.php

Despite collectors are event listeners, they aren't doing anything when Debugger::$skipCollect is true so that is already pretty lightweight.

If we want to completely skip calling listeners, own implementation of ListenerProviderInterface registered along with standard provider via CompositeProvider is needed. There, we'll have a dependency on Debugger and will be able to get $skipCollect and return an empty array of listeners in case it is true.

Disadvantage of such approach is that it requires event dispatcher to always use composite provider.

Disabling proxies

As for proxies, can we make ContainerInterfaceProxy dependent on Debugger and aware of $skipCollect? If yes, we can call $this->decorate->get() in this case.

[xepozz]

@samdark

Events

You're almost right. When a new request starts the application middleware stack, it's being checked by the Debugger. If any conditions are met, then the Debugger does not start collectors, and each collector that's checked isActive inside itself finishes processing immediately, so almost a free check.

Proxies

In long-running applications such as RoadRunner, etc., the container state within any related and resolved services does not change from request to request. So pushing a "debug" logger/event dispatcher into a user service may be stuck until a full application restart (= web server/worker restart), which is totally not a good idea.

We can add skipping logic into proxies, so then the "debug" logger can skip any proxy functionality and call directly the decorated object. Like the following:

public function __call($method, $params) {
    if ($this->disabled) {
        $this->decorated->$method(...$params);
    }

    $this->$method(...$params);
}

But these checks must be added to each decorated function.

And one more thing: going deeper, the logger/event dispatcher may be stuck in a not decorated state, so any debugger's run will skip calling the "debug" correspondent classes => we must register "debug" proxies at all times even if the debugger did not activate the debug session.

[samdark]

So no need to do anything about events for release. As for proxies... yes, seems we have to add "disabled" to all of them...

[xepozz]

I'll try to figure it out