When disabling the profiler programmatically, collectors will not be called, but any collaborating services that collect profiling data on behalf of those collectors will still do unnecessary work. By being able to inquire about whether the profiler is enabled, such services may elect to omit data collection thus reducing memory and CPU footprint.

Relates to#45241.

carsonbot added Status: Needs Review Feature labels

Feb 1, 2022

carsonbot added this to the6.1 milestone

Feb 1, 2022

Bilge force-pushed theprofiler-enabled branch 2 times, most recently from2063db8 toc3b08e8Compare

February 1, 2022 10:08

Copy link

carsonbot commentedFeb 2, 2022

Hey!

I think@mtarld has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

Copy link

Member

GromNaN commentedFeb 2, 2022•
edited
Loading

If I get it right, the "collaborating services" will depend on theprofiler service, which is not needed currently. Don't you get a circular depencency (profiler -> collector -> collaborating service -> profiler) ?

Alternative proposition : propagate status to the collectors whenProfiler::disable() is called.

Copy link

ContributorAuthor

Bilge commentedFeb 2, 2022 via email

This has nothing to do with collectors.

…

On Wed, 2 Feb 2022, 07:37 Jérôme Tamarelle, ***@***.***> wrote: If I get it right, the "collaborating services" will depends on the profiler service, which is not needed currently. You don't get a circular depencency (profiler -> collector -> collaborating service -> profiler) ? Alternative proposition : propagate status to the collectors when Profiler::disable() is called. — Reply to this email directly, view it on GitHub <#45265 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADS4YQOR6R6E7HS77YZR6LUZDNLXANCNFSM5NIVTLCA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

Copy link

Member

GromNaN commentedFeb 2, 2022

This has nothing to do with collectors.

Well, could you describe your use-case a little more. What is a "collaborating service that collect profiling data on behalf of those collectors" ?

Copy link

ContributorAuthor

Bilge commentedFeb 2, 2022•
edited by GromNaN
Loading

Sure. I wrote a database profiler similar to the one you know in Doctrine, but for theAmp drivers instead (see also:doctrine/DoctrineBundle#1464). The regular connection pool is decorated with a profiled pool; this is the collaborating service. This service is injected into the collector. When the profiler is disabled the collector will not be called, but the pool is still decorated and still collects data anyway, since it is unaware of the status of the profiler.

GromNaN approved these changes

Feb 2, 2022

View reviewed changes

Copy link

Member

GromNaN left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the details.
I have almost the same use-case and used%kernel.debug% to enable/disable collection of queries in the client decorator. Your solution is smarter.

Regarding circular dependency, the container builder is able to handle it.