r50091 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r50090‎ \| r50091 \| r50092 >
Date:	05:23, 1 May 2009
Author:	tstarling
Status:	deferred
Tags:
Comment:	Backported r49068, r49086: change tagging performance fixes.
Modified paths:	/branches/REL1_15/phase3 (modified) (history) /branches/REL1_15/phase3/includes/ChangeTags.php (modified) (history) /branches/REL1_15/phase3/includes/LogEventsList.php (modified) (history) /branches/REL1_15/phase3/includes/PageHistory.php (modified) (history) /branches/REL1_15/phase3/includes/specials/SpecialContributions.php (modified) (history) /branches/REL1_15/phase3/includes/specials/SpecialNewpages.php (modified) (history) /branches/REL1_15/phase3/includes/specials/SpecialRecentchanges.php (modified) (history) /branches/REL1_15/phase3/includes/specials/SpecialRecentchangeslinked.php (modified) (history) /branches/REL1_15/phase3/includes/specials/SpecialWatchlist.php (modified) (history)

Diff [purge]

Index: branches/REL1_15/phase3/includes/ChangeTags.php
—	—	@@ -90,7 +90,8 @@
91	91	* Handles selecting tags, and filtering.
92	92	* Needs $tables to be set up properly, so we can figure out which join conditions to use.
93	93	*/
94		~~- static function modifyDisplayQuery( &$tables, &$fields, &$conds, &$join_conds, $filter_tag = false ) {~~
	94	+ static function modifyDisplayQuery( &$tables, &$fields, &$conds,
	95	+ &$join_conds, &$options, $filter_tag = false ) {
95	96	global $wgRequest, $wgUseTagFilter;
96	97
97	98	if ($filter_tag === false) {
—	—	@@ -118,6 +119,9 @@
119	120	// Somebody wants to filter on a tag.
120	121	// Add an INNER JOIN on change_tag
121	122
	123	+ // FORCE INDEX -- change_tags will almost ALWAYS be the correct query plan.
	124	+ $options['USE INDEX'] = array( 'change_tag' => 'change_tag_tag_id' );
	125	+ unset( $options['FORCE INDEX'] );
122	126	$tables[] = 'change_tag';
123	127	$join_conds['change_tag'] = array( 'INNER JOIN', "ct_$join_cond=$join_cond" );
124	128	$conds['ct_tag'] = $filter_tag;
—	—	@@ -176,4 +180,4 @@
177	181	$wgMemc->set( $key, $emptyTags, 300 );
178	182	return $emptyTags;
179	183	}
180		-}
\ No newline at end of file
	184	+}
Index: branches/REL1_15/phase3/includes/LogEventsList.php
—	—	@@ -655,7 +655,7 @@
656	656	);
657	657
658	658	ChangeTags::modifyDisplayQuery( $info['tables'], $info['fields'], $info['conds'],
659		~~- $info['join_conds'], $this->mTagFilter );~~
	659	+ $info['join_conds'], $info['options'], $this->mTagFilter );
660	660
661	661	return $info;
662	662	}
Index: branches/REL1_15/phase3/includes/specials/SpecialRecentchangeslinked.php
—	—	@@ -76,6 +76,7 @@
77	77	$tables = array( 'recentchanges' );
78	78	$select = array( $dbr->tableName( 'recentchanges' ) . '.*' );
79	79	$join_conds = array();
	80	+ $query_options = array();
80	81
81	82	// left join with watchlist table to highlight watched rows
82	83	if( $uid = $wgUser->getId() ) {
—	—	@@ -84,7 +85,8 @@
85	86	$join_conds['watchlist'] = array( 'LEFT JOIN', "wl_user={$uid} AND wl_title=rc_title AND wl_namespace=rc_namespace" );
86	87	}
87	88
88		~~- ChangeTags::modifyDisplayQuery( $tables, $select, $conds, $join_conds, $opts['tagfilter'] );~~
	89	+ ChangeTags::modifyDisplayQuery( $tables, $select, $conds, $join_conds,
	90	+ $query_options, $opts['tagfilter'] );
89	91
90	92	// XXX: parent class does this, should we too?
91	93	// wfRunHooks('SpecialRecentChangesQuery', array( &$conds, &$tables, &$join_conds, $opts ) );
—	—	@@ -142,7 +144,7 @@
143	145	$select,
144	146	$conds + $subconds,
145	147	__METHOD__,
146		~~- array( 'ORDER BY' => 'rc_timestamp DESC', 'LIMIT' => $limit ),~~
	148	+ array( 'ORDER BY' => 'rc_timestamp DESC', 'LIMIT' => $limit ) + $query_options,
147	149	$join_conds + array( $link_table => array( 'INNER JOIN', $subjoin ) )
148	150	);
149	151	}
Index: branches/REL1_15/phase3/includes/specials/SpecialNewpages.php
—	—	@@ -438,7 +438,12 @@
439	439	$fields = array();
440	440
441	441	## Modify query for tags
442		~~- ChangeTags::modifyDisplayQuery( $info['tables'], $fields, $info['conds'], $info['join_conds'], $this->opts['tagfilter'] );~~
	442	+ ChangeTags::modifyDisplayQuery( $info['tables'],
	443	+ $fields,
	444	+ $info['conds'],
	445	+ $info['join_conds'],
	446	+ $info['options'],
	447	+ $this->opts['tagfilter'] );
443	448
444	449	return $info;
445	450	}
Index: branches/REL1_15/phase3/includes/specials/SpecialRecentchanges.php
—	—	@@ -270,6 +270,7 @@
271	271
272	272	$tables = array( 'recentchanges' );
273	273	$join_conds = array();
	274	+ $query_options = array( 'USE INDEX' => array('recentchanges' => 'rc_timestamp') );
274	275
275	276	$uid = $wgUser->getId();
276	277	$dbr = wfGetDB( DB_SLAVE );
—	—	@@ -287,17 +288,25 @@
288	289	}
289	290
290	291	// Tag stuff.
291		~~- $fields = array(); // Fields are * in this case, so let the function modify an empty array to keep it happy.~~
292		~~- ChangeTags::modifyDisplayQuery( $tables, $fields, $conds, $join_conds, $opts['tagfilter'] );~~
	292	+ $fields = array();
	293	+ // Fields are * in this case, so let the function modify an empty array to keep it happy.
	294	+ ChangeTags::modifyDisplayQuery( $tables,
	295	+ $fields,
	296	+ $conds,
	297	+ $join_conds,
	298	+ $query_options,
	299	+ $opts['tagfilter']
	300	+ );
293	301
294	302	wfRunHooks('SpecialRecentChangesQuery', array( &$conds, &$tables, &$join_conds, $opts ) );
295	303
296	304	// Is there either one namespace selected or excluded?
	305	+ // Tag filtering also has a better index.
297	306	// Also, if this is "all" or main namespace, just use timestamp index.
298		~~- if( is_null($namespace) \|\| $invert \|\| $namespace == NS_MAIN ) {~~
	307	+ if( is_null($namespace) \|\| $invert \|\| $namespace == NS_MAIN \|\| $opts['tagfilter'] ) {
299	308	$res = $dbr->select( $tables, '*', $conds, __METHOD__,
300		~~- array( 'ORDER BY' => 'rc_timestamp DESC', 'LIMIT' => $limit,~~
301		~~- 'USE INDEX' => array('recentchanges' => 'rc_timestamp') ),~~
	309	+ array( 'ORDER BY' => 'rc_timestamp DESC', 'LIMIT' => $limit ) +
	310	+ $query_options,
302	311	$join_conds );
303	312	// We have a new_namespace_time index! UNION over new=(0,1) and sort result set!
304	313	} else {
Index: branches/REL1_15/phase3/includes/specials/SpecialContributions.php
—	—	@@ -412,8 +412,12 @@
413	413	'join_conds' => $join_cond
414	414	);
415	415
416		~~- ChangeTags::modifyDisplayQuery( $queryInfo['tables'], $queryInfo['fields'], $queryInfo['conds'],~~
417		~~- $queryInfo['join_conds'], $this->tagFilter );~~
	416	+ ChangeTags::modifyDisplayQuery( $queryInfo['tables'],
	417	+ $queryInfo['fields'],
	418	+ $queryInfo['conds'],
	419	+ $queryInfo['join_conds'],
	420	+ $queryInfo['options'],
	421	+ $this->tagFilter );
418	422
419	423	wfRunHooks( 'ContribsPager::getQueryInfo', array( &$this, &$queryInfo ) );
420	424	return $queryInfo;
Index: branches/REL1_15/phase3/includes/specials/SpecialWatchlist.php
—	—	@@ -219,7 +219,7 @@
220	220	$join_conds['page'] = array('LEFT JOIN','rc_cur_id=page_id');
221	221	}
222	222
223		~~- ChangeTags::modifyDisplayQuery( $tables, $fields, $conds, $join_conds, '' );~~
	223	+ ChangeTags::modifyDisplayQuery( $tables, $fields, $conds, $join_conds, $options, '' );
224	224	wfRunHooks('SpecialWatchlistQuery', array(&$conds,&$tables,&$join_conds,&$fields) );
225	225
226	226	$res = $dbr->select( $tables, $fields, $conds, __METHOD__, $options, $join_conds );
Index: branches/REL1_15/phase3/includes/PageHistory.php
—	—	@@ -572,7 +572,12 @@
573	573	'options' => array( 'USE INDEX' => array('revision' => 'page_timestamp') ),
574	574	'join_conds' => array( 'tag_summary' => array( 'LEFT JOIN', 'ts_rev_id=rev_id' ) ),
575	575	);
576		~~- ChangeTags::modifyDisplayQuery( $queryInfo['tables'], $queryInfo['fields'], $queryInfo['conds'], $queryInfo['join_conds'], $this->tagFilter );~~
	576	+ ChangeTags::modifyDisplayQuery( $queryInfo['tables'],
	577	+ $queryInfo['fields'],
	578	+ $queryInfo['conds'],
	579	+ $queryInfo['join_conds'],
	580	+ $queryInfo['options'],
	581	+ $this->tagFilter );
577	582	wfRunHooks( 'PageHistoryPager::getQueryInfo', array( &$this, &$queryInfo ) );
578	583	return $queryInfo;
579	584	}
Property changes on: branches/REL1_15/phase3
___________________________________________________________________
Name: svn:mergeinfo
580	585	+ /trunk/phase3:49068,49086

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r49068	Fix tag filtering by adding a FORCE INDEX to the relevant queries	werdna	13:01, 31 March 2009
r49086	Fixed fatal from r49068. Removed verbose formatting :)	aaron	15:57, 31 March 2009

Status & tagging log

07:09, 19 May 2009 Tim Starling (talk | contribs) changed the status of r50091 [removed: new added: deferred]
23:38, 1 May 2009 Brion VIBBER (talk | contribs) changed the status of r50091 [removed: new added: deferred]