r105123 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r105122‎ | r105123 | r105124 >
Date:18:35, 4 December 2011
Author:johnduhart
Status:resolved (Comments)
Tags:todo 
Comment:
svn:eol-style native on all phase3 stuff, since I messed that up in r105122
Also adding the MWDebug class I missed there as well
Modified paths:
  • /trunk/phase3/includes/debug (added) (history)
  • /trunk/phase3/includes/debug/Debug.php (added) (history)
  • /trunk/phase3/includes/upload/UploadFromChunks.php (modified) (history)
  • /trunk/phase3/maintenance/archives/patch-uploadstash_chunk.sql (modified) (history)
  • /trunk/phase3/maintenance/dev/router.php (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.debug.css (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.debug.js (modified) (history)

Diff [purge]

Property changes on: trunk/phase3/maintenance/archives/patch-uploadstash_chunk.sql
___________________________________________________________________
Added: svn:eol-style
11 + native
Property changes on: trunk/phase3/maintenance/dev/router.php
___________________________________________________________________
Added: svn:eol-style
22 + native
Property changes on: trunk/phase3/includes/upload/UploadFromChunks.php
___________________________________________________________________
Added: svn:eol-style
33 + native
Index: trunk/phase3/includes/debug/Debug.php
@@ -0,0 +1,216 @@
 2+<?php
 3+
 4+/**
 5+ * New debugger system that outputs a toolbar on page view
 6+ *
 7+ * @todo Clean up HTML generated by the javascript
 8+ * @todo Profiler support
 9+ */
 10+class MWDebug {
 11+
 12+ /**
 13+ * Log lines
 14+ *
 15+ * @var array
 16+ */
 17+ protected static $log = array();
 18+
 19+ /**
 20+ * Debug messages from wfDebug()
 21+ *
 22+ * @var array
 23+ */
 24+ protected static $debug = array();
 25+
 26+ /**
 27+ * Queries
 28+ *
 29+ * @var array
 30+ */
 31+ protected static $query = array();
 32+
 33+ /**
 34+ * Request information
 35+ *
 36+ * @var array
 37+ */
 38+ protected static $request = array();
 39+
 40+ /**
 41+ * Is the debugger enabled?
 42+ *
 43+ * @var bool
 44+ */
 45+ protected static $enabled = true;
 46+
 47+ /**
 48+ * Called in Setup.php, initializes the debugger if it is enabled with
 49+ * $wgDebugToolbar
 50+ */
 51+ public static function init() {
 52+ global $wgDebugToolbar;
 53+
 54+ if ( !$wgDebugToolbar ) {
 55+ self::$enabled = false;
 56+ return;
 57+ }
 58+
 59+ RequestContext::getMain()->getOutput()->addModules( 'mediawiki.debug' );
 60+ }
 61+
 62+ /**
 63+ * Adds a line to the log
 64+ *
 65+ * This does nothing atm, there's not frontend for it
 66+ *
 67+ * @todo Add error and warning log type
 68+ * @todo Add support for passing objects
 69+ *
 70+ * @param $str string
 71+ */
 72+ public static function log( $str ) {
 73+ if ( !self::$enabled ) {
 74+ return;
 75+ }
 76+
 77+ self::$log[] = $str;
 78+ }
 79+
 80+ /**
 81+ * This is a method to pass messages from wfDebug to the pretty debugger.
 82+ * Do NOT use this method, use MWDebug::log or wfDebug()
 83+ *
 84+ * @param $str string
 85+ */
 86+ public static function debugMsg( $str ) {
 87+ if ( !self::$enabled ) {
 88+ return;
 89+ }
 90+
 91+ self::$debug[] = trim( $str );
 92+ }
 93+
 94+ /**
 95+ * Begins profiling on a database query
 96+ *
 97+ * @param $sql string
 98+ * @param $function string
 99+ * @param $isMaster bool
 100+ * @return int ID number of the query to pass to queryTime or -1 if the
 101+ * debugger is disabled
 102+ */
 103+ public static function query( $sql, $function, $isMaster ) {
 104+ if ( !self::$enabled ) {
 105+ return -1;
 106+ }
 107+
 108+ self::$query[] = array(
 109+ 'sql' => $sql,
 110+ 'function' => $function,
 111+ 'master' => (bool) $isMaster,
 112+ 'time' > 0.0,
 113+ '_start' => microtime( true ),
 114+ );
 115+
 116+ return count( self::$query ) - 1;
 117+ }
 118+
 119+ /**
 120+ * Calculates how long a query took.
 121+ *
 122+ * @param $id int
 123+ */
 124+ public static function queryTime( $id ) {
 125+ if ( $id === -1 || !self::$enabled ) {
 126+ return;
 127+ }
 128+
 129+ self::$query[$id]['time'] = microtime( true ) - self::$query[$id]['_start'];
 130+ unset( self::$query[$id]['_start'] );
 131+ }
 132+
 133+ /**
 134+ * Processes a WebRequest object
 135+ *
 136+ * @param $request WebRequest
 137+ */
 138+ public static function processRequest( WebRequest $request ) {
 139+ if ( !self::$enabled ) {
 140+ return;
 141+ }
 142+
 143+ self::$request = array(
 144+ 'method' => $_SERVER['REQUEST_METHOD'],
 145+ 'url' => $request->getRequestURL(),
 146+ 'headers' => $request->getAllHeaders(),
 147+ 'params' => $request->getValues(),
 148+ );
 149+ }
 150+
 151+ /**
 152+ * Returns a list of files included, along with their size
 153+ *
 154+ * @return array
 155+ */
 156+ protected static function getFilesIncluded() {
 157+ $files = get_included_files();
 158+ $fileList = array();
 159+ foreach ( $files as $file ) {
 160+ $size = filesize( $file );
 161+ $fileList[] = array(
 162+ 'name' => $file,
 163+ 'size' => self::formatBytes( $size ),
 164+ );
 165+ }
 166+
 167+ return $fileList;
 168+ }
 169+
 170+ /**
 171+ * Returns the HTML to add to the page for the toolbar
 172+ *
 173+ * @return string
 174+ */
 175+ public static function getDebugHTML() {
 176+ if ( !self::$enabled ) {
 177+ return '';
 178+ }
 179+
 180+ global $wgVersion, $wgRequestTime;
 181+ $debugInfo = array(
 182+ 'mwVersion' => $wgVersion,
 183+ 'phpVersion' => PHP_VERSION,
 184+ 'time' => microtime( true ) - $wgRequestTime,
 185+ 'log' => self::$log,
 186+ 'debugLog' => self::$debug,
 187+ 'queries' => self::$query,
 188+ 'request' => self::$request,
 189+ 'memory' => self::formatBytes( memory_get_usage() ),
 190+ 'memoryPeak' => self::formatBytes( memory_get_peak_usage() ),
 191+ 'includes' => self::getFilesIncluded(),
 192+ );
 193+ // TODO: Clean this up
 194+ $html = Html::openElement( 'script' );
 195+ $html .= 'var debugInfo = ' . Xml::encodeJsVar( $debugInfo ) . ';';
 196+ $html .= " $(function() { mw.loader.using( 'mediawiki.debug', function() { mw.Debug.init( debugInfo ) } ); }); ";
 197+ $html .= Html::closeElement( 'script' );
 198+
 199+ return $html;
 200+ }
 201+
 202+ /**
 203+ * Formats raw bytes integer into a human readable format
 204+ *
 205+ * @author John Himmelman - http://stackoverflow.com/a/2510540/343911
 206+ * @param $size int
 207+ * @param $precision int
 208+ * @return string
 209+ */
 210+ protected static function formatBytes( $size, $precision = 2 ) {
 211+ $base = log( $size ) / log( 1024 );
 212+ // If we ever use 1TB of RAM we're fucked
 213+ $suffixes = array( '', 'kb', 'MB', 'GB', 'TB' );
 214+
 215+ return round( pow( 1024, $base - floor( $base ) ), $precision ) . $suffixes[floor( $base )];
 216+ }
 217+}
\ No newline at end of file
Property changes on: trunk/phase3/includes/debug/Debug.php
___________________________________________________________________
Added: svn:eol-style
1218 + native
Property changes on: trunk/phase3/resources/mediawiki/mediawiki.debug.css
___________________________________________________________________
Added: svn:eol-style
2219 + native
Property changes on: trunk/phase3/resources/mediawiki/mediawiki.debug.js
___________________________________________________________________
Added: svn:eol-style
3220 + native

Follow-up revisions

RevisionCommit summaryAuthorDate
r106306Followup r105122 & r105123, fixes and improvements per CR...johnduhart02:26, 15 December 2011
r109032Move MWDebug enabling logic to Setup.php...hashar13:44, 16 January 2012
r112021Followup r105123, fix for MWDebug logging SQL queries on command line modejohnduhart16:48, 21 February 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r105122Adding new debugging toolbar...johnduhart18:29, 4 December 2011

Comments

#Comment by Nikerabbit (talk | contribs)   06:36, 5 December 2011

Is Language::formatSize not usable replacement for your formatBytes?

#Comment by Reedy (talk | contribs)   01:20, 13 December 2011

The formatBytes here looks better IMHO...

To match functionality completely, it does need Tera, see bug 33014


I unmarked it fixme due to unknown reason of being set

#Comment by Johnduhart (talk | contribs)   02:24, 15 December 2011

But are there i18n issues with this?

Then again none of this is translated.

#Comment by P858snake (talk | contribs)   06:18, 10 December 2011

Brion, Why did you FIXME this? because of Nike's comment?

#Comment by Hashar (talk | contribs)   10:33, 12 December 2011

Do we really want that to be in core? I would prefer having an extension for this debugging tool.

#Comment by Johnduhart (talk | contribs)   00:13, 13 December 2011

I disagree. Developers would benefit from a ready to go tool that's built into core to help them out. Additionally, a lot of the profiling/debugging code for this would be tricky to pull off with an extension.

#Comment by Nikerabbit (talk | contribs)   07:34, 15 December 2011

Should not use Title Case String. The JavaScript causes errors in IE6 and IE7.

#Comment by Johnduhart (talk | contribs)   17:48, 2 January 2012

Huh?

#Comment by Aaron Schulz (talk | contribs)   21:51, 13 January 2012

$wgDebugToolbar should have a path-disclosure comment in DefaultSettings.

#Comment by Tim Starling (talk | contribs)   04:58, 16 February 2012

This will store the SQL text of every SQL query done, even in CLI mode, if $wgDebugToolbar is enabled. Maintenance scripts which write a lot of data will eventually send the server into swap. Please limit it, or disable it in CLI mode.

#Comment by Hashar (talk | contribs)   08:39, 16 February 2012

Maybe we should check against $wgDebugDumpSql ? One might want to log the queries of a maintenance script.

#Comment by Tim Starling (talk | contribs)   09:55, 16 February 2012

Despite the name, this MWDebug class doesn't handle debug logging.

Status & tagging log