Page MenuHomePhabricator

Decouple Profiler class from WebRequest and RequestContext
Closed, ResolvedPublic

Description

The Profiler.php class, when called upon to process any profiler outputs at the end of a web request, requires a RequestContext. This is especially problematic for Maintenance CLI process as it somewhat unpredictable to construct a RequestContext there, and is probably one of very few cases where that currently happens for CLI processes.

The Profiler::getContext method guards against this by falling back to RequestContext::getMain() and emitting a warning. But, this is never reached because the only real caller to Profiler::logData is wfLogProfilingData which helpfully does this:

wfLogProfilingData
	$profiler = Profiler::instance();
	$profiler->setContext( RequestContext::getMain() );

.. which is called at the end of web requests (MediaWiki::restInPeace), but also CLI processes (Maintenance::shutdown).

The only thing Profiler actually uses this for is for this one call:

Profiler.php
$this->getContext()->getRequest()->getElapsedTime()

And all that ultimately does is, is run this plain PHP statement:

WebRequest.php
microtime( true ) - $_SERVER['REQUEST_TIME_FLOAT']

Event Timeline

Change 725152 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] Deprecate wfLogProfilingData(), improve statsd/profiling docs

https://gerrit.wikimedia.org/r/725152

Change 725440 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] profiler: Remove dependency on WebRequest from Profiler

https://gerrit.wikimedia.org/r/725440

Change 725152 merged by jenkins-bot:

[mediawiki/core@master] Deprecate wfLogProfilingData(), improve statsd/profiling docs

https://gerrit.wikimedia.org/r/725152

Krinkle triaged this task as Medium priority.Oct 10 2021, 12:20 AM
Krinkle moved this task from Inbox to Backlog: Maintenance on the Performance-Team board.

I'm taking this on in order to allow for T240685 to take a cleaner approach for its integration without having to inherit or workaround the tech debt that grew into the current Statsd component.

Change 725440 merged by jenkins-bot:

[mediawiki/core@master] profiler: Remove dependency on WebRequest from Profiler

https://gerrit.wikimedia.org/r/725440

Change 733135 had a related patch set uploaded (by Krinkle; author: Zabe):

[mediawiki/core@master] Add deprecation warnings to wfLogProfilingData()

https://gerrit.wikimedia.org/r/733135

Change 733135 merged by jenkins-bot:

[mediawiki/core@master] Add deprecation warnings to wfLogProfilingData()

https://gerrit.wikimedia.org/r/733135