Profiler::instance() contains some global state by using $wgProfiler
Also WikiMap::getCurrentWikiDbDomain() (maybe use LoadBalancer instead)
Also RequestContext and LoggerFactory
Profiler::instance() contains some global state by using $wgProfiler
Also WikiMap::getCurrentWikiDbDomain() (maybe use LoadBalancer instead)
Also RequestContext and LoggerFactory
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Open | None | T259960 Inject services into API modules and special pages | |||
Resolved | Krinkle | T265398 Use DI for Profiler class instance in MediaWiki core code |
Using DI for this makes sense to me. Thanks. I would recommend against making it a service, though, since this is not meant to be tied to a specific wiki or other other swappable state.
Rather, it should be global to the PHP process, independent of potentially multiple service containers (e.g. once we start to support multiple/foreign wiki contexts, which I see as one of the primary purposes for service wiring to exist). As such, I think Profiler::instance() remains the appropiate entry point to this singleton, but can indeed be injected into serviced classes (or used directly, in non-service classes and entry point code).
I think it's important that as we move toward DI in this class, that we make construction remains cheap. This is naturally the case in the legacy/global code as things are lazily discovered and initialised. I have seen this done incorrectly before in other classes, so I want to make this is taken into account by first fixing any expensive constructors of dependencies. Especially here since this may very well be the earliest and most constructed singleton we have that has dependencies on lots of other major singletons.
For wgProfiler and db name, it might make sense to take those directly as setting values without involving any indirection other needless complexity. They are not traditional configuration variables and would be well within the subset of config variables required for bootstrapping (like db host/pwd) and thus not loadable from an external source by extensions or otherwise benefit from Config. For testing the same constructor can be used directly as we do with service classes.
Fixed in:
profiler: Inject $wgProfiler from Setup.php to Profiler
https://gerrit.wikimedia.org/r/c/mediawiki/core/+/809043