Page MenuHomePhabricator

Remove ProfilerOutputDb feature from MediaWiki core
Closed, ResolvedPublic

Description

This feature encompasses:

  • ... the (optional) profiling database table, created by maintenance/archives/patch-profiling.sql,
  • ... the ProfilerOutputDb class implementation of ProfilerOutput, as configurable through $wgProfiler['output'] = ProfilerOutputDb::class or $wgProfiler['output'] = 'db',
  • ... the profileinfo.php web entry point, its HTML5 user interface, and the $wgEnableProfileInfo configuration variable.
Rationale

As far as I'm aware, this feature has not been in use by either WMF, nor any MW developers, for a long time.

The last time I got it to work locally was in 2012. I also tried it once more around 2014 when we did some related development to the Profiling systems in MW in preparation for XHProf/HHVM, however even those changes were never tested as far as I know.

For development use cases, where the user viewing the page is the developer, we have already ProfilerOutputText instead which appends the captured performance analysis profile to the output of the web request (in an HTML or JS/CSS comment), or to the output of a maintenance script. The ProfilerOutputDb implementation and profileinfo.php are basically a nicer version of that, but I don't think we need that for developers.

For production use cases, I don't think ProfilerOutputDb is intended for. We did use to have another implementation of ProfilerOutput that sent the captured data to to central point: ProfilerOutputUdp. This class was already removed from core in 2015 with 660a7759 / https://gerrit.wikimedia.org/r/247971. For production, you'd want the profiling data to be sent from the web server to a central location to be looked at by other developers, but there are much better systems out there for sampling, storing, and viewing this kind of information. At WMF, we've been using Arc Lamp and XHGui since 2015.

I don't think the problem space of storing PHP profiles and providing a UI to analyse them, should be one solved by MediaWiki core or otherwise maintained by WMF. (And the one we have is almost certainly not one that I think anyone would want to use in production, anyway).

Timeline of last 7 years in development

Event Timeline

Krinkle created this task.Aug 27 2019, 5:43 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 27 2019, 5:43 PM
Krinkle updated the task description. (Show Details)Sep 3 2019, 8:16 PM
Krinkle moved this task from Unsorted to Needs removal on the Technical-Debt board.
Krinkle removed a project: Technical-Debt.

@WDoranWMF Forgot to say why I tagged, sorry. This feature doesn't have any steward and seems closely related to code we maintain. So as part of taking this on, we're also intending to sunset part of it.

I wanted to know whether CPT has any reservations about this feature, possible concerns, or knows about anyone using it – before we go ahead with its removal.

@Krinkle We can move this into Clinic Duty and get an evaluation. The PMs have signed off, we just need to review to make sure there aren't any usecases we know of. What's the urgency for this?

@WDoranWMF Would like to have it dropped from the 1.34 release (if we decide not to keep it), so as to avoid maintenance for it going forward in the next 6-12 months.

The last day of the master branch for 1.34.0 will be cut Mon 7 October, but there are back-porting opportunities until end of October (although the later we drop it, the greater the chances of beta and release candidate testing not uncovering potential issues or concerns).

Krinkle claimed this task.Sep 24 2019, 3:57 AM
Krinkle triaged this task as Medium priority.
Krinkle removed Krinkle as the assignee of this task.Oct 1 2019, 4:57 PM
Krinkle moved this task from Blocked or Needs-CR to Inbox on the Performance-Team board.
Anomie added a subscriber: Anomie.Oct 10 2019, 1:52 PM

@WDoranWMF Forgot to say why I tagged, sorry. This feature doesn't have any steward and seems closely related to code we maintain. So as part of taking this on, we're also intending to sunset part of it.

I wanted to know whether CPT has any reservations about this feature, possible concerns, or knows about anyone using it – before we go ahead with its removal.

I personally have no concerns about removing it, and I don't know of anyone using it. I don't see any references to it in extensions or skins in Gerrit. Core mentions it in a doc comment in DefaultSettings.php, and there's some schema update code from MW 1.13 for adding a column to the optional table if it's present (and code in DatabaseSqliteTest to test the upgrade).

I'll leave it to @Krinkle to decide whether to apply https://www.mediawiki.org/wiki/Deprecation_policy#Removal_without_deprecation, and to justify it if so. Again, I have no objection.

Change 542620 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] profiler: Deprecate ProfilerOutputDb and profileinfo.php

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

Krinkle claimed this task.EditedOct 11 2019, 10:56 PM

@Anomie Thanks. It'll be deprecated in 1.34 (not without deprecation), hence the urgency :)

Change 542620 merged by jenkins-bot:
[mediawiki/core@master] profiler: Deprecate ProfilerOutputDb and profileinfo.php

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

Change 543207 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@REL1_34] profiler: Deprecate ProfilerOutputDb and profileinfo.php

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

Krinkle closed this task as Resolved.Oct 15 2019, 8:03 PM
Krinkle reopened this task as Open.
Krinkle removed Krinkle as the assignee of this task.
Krinkle removed a project: Patch-For-Review.

Re-opening since the "removal" part is still outstanding and will happen sometime this quarter during the 1.35 cycle.

Change 543207 merged by jenkins-bot:
[mediawiki/core@REL1_34] profiler: Deprecate ProfilerOutputDb and profileinfo.php

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

Krinkle added a comment.EditedOct 16 2019, 10:38 PM

Next step: In master (1.35):

  • Remove the ProfilerOutputDb class,
  • Remove the profileinfo.php entry point.
  • Remove the profiling table schema definitions. (Document in the release notes that given it is mainly a developer feature, and the schema wasn't auto-created, it will also not be auto-removed. Anyone wishing to remove their old profiles can do so by running the DROP TABLE profiling; statement through sql.)
  • Remove anything else internal that solely existed for making ProfilerOutputDb possible.

And, update documentation:

Krinkle claimed this task.Oct 19 2019, 8:16 PM

Change 545308 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] profiler: Remove ProfilerOutputDb and profileinfo.php entry point

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

Change 545308 merged by jenkins-bot:
[mediawiki/core@master] profiler: Remove ProfilerOutputDb and profileinfo.php entry point

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