I've been tracking a performance problem which took me into this
profiler autoloading code by way of a false trail, so I am ignorant of
the development history, but at least I have come to this with fresh
eyes. I have also trawled the DL for relevant threads, and my topic is
related to a thread "Dropping StartProfiler.php", 25-28 Dec 2011,
largely between Daniel Friesen and Tim Starling. (I've just subscribed
to the DL, so can't pick up the thread references).
I have some general observations:
1) Profiling is a rare activity undertaken by developers and sysadmins,
so given the autoload architecture, it makes a load of sense simply to
not load it at all for normal mediawiki use. The use of the functions
wfProfileIn and wfProfileOut facilitate this.
2) However, at the moment db/Database.php (793) and other routines
invoke Profiler::instance() unconditionally which causes at a minimum
ProfilerStub instantiation.
3) The profiling architecture allows the binding of context parameters
during the instantiation of profiling extension classes, yet the
configuration by setting of the array $wgProfiler[] does not enable
these to be passed to the invoked $wgProfiler['class'] class.
4) Whether singleton templates are recommended is at least controversial
topic. Nonetheless, MW does use a number of classic singletons, for
example MessageCache. However, IMO, the Profiler class is an anomaly in
that it is sort of a singleton hybrid -- see instance() and
setInstance() methods. Doing this is confusing and seems to add zero value.
5) As discussed by Daniel in the above referenced thread, the only
modules included between Profiler and LocalSettings are Defines,
StartProfiler and DefaultSettings. Bypassing StartProfiller, Defines
and DefaultSettings set configuration defines and global variable, so
little is to be gained in profiling them.
And so to my suggestions:
A) Modify WebStart to remove all Profiler class related requires and
move the wfProfileIn( 'WebStart.php-conf' ) statement below the
LocalSettings load
B) Move wfProfileIn() and wfProfileOut() to GlobalFunctions, since this
is truly what they are.
C) Add an additional wfProfileEnabled() to GlobalFunctions which guards
any reference to Profile or its extended classes by a test of
$wgProfiler. Replace the ungarded Profiler::instance()->isSub() in
db/Database.php et al by a wfProfileEnabled() test. This plus (B) means
that the Profiler classes will not be autoloaded at all under normal
circumstances.
D) Profiling can now simply be enabled by setting
$wfProfiler = new ProfilerWhateverWanted( ... whatever params are
wanted ... )
in LocalSettings. The profiler extension and base class will then be
loaded by the autoloader except in the case of a custom profiler which
will need an explicit require. Forget the idea of using an array format
for $wfProfiler. This seems to have been for dogmatic reasons and isn't
fully implemented anyway.
E) If dynamic enabling / disabling of profiling is needed to achieve
timeline windowing then this can be done easily by setting, say
$wfProfilerTemp to the profiler extension class instance and setting the
$wfProfiler global to this variable as required and unsetting ditto.
F) Hence the guard test in wfProfileIn(), wfProfileOut() and
wfProfileEnabled() becomes the simple and lean (function call + 10
opcodes according to VLD):
if (!isset( $wfProfiler ) return;
G) Make Profiler a simple (non-pseudo singleton) class as it will be
autoload as the base class of any specific Profiler extension . As
$wfProfiler is used to contain the global profiler extension class
instance then, instance() and setInstance() add no value.
I can provide my patch which implements this, if wanted. Comments?
Regards Terry Ellison