r70970 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r70969‎ | r70970 | r70971 >
Date:17:10, 12 August 2010
Author:demon
Status:ok (Comments)
Tags:
Comment:
Get rid of StubUser. Constructing a user object isn't quite as intensive as it once was. It actually takes more time using the StubUser (construction + unstub on first call) than just initializing User::newFromSession() from the start. Either way, the real overhead starts when you start calling methods (for the nitpicky, the optimization was only about 20µs. The real gain here was eliminating a StubObject)
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/Setup.php (modified) (history)
  • /trunk/phase3/includes/StubObject.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/StubObject.php
@@ -165,30 +165,3 @@
166166 }
167167 }
168168 }
169 -
170 -/**
171 - * Stub object for the user. The initialisation of the will depend of
172 - * $wgCommandLineMode. If it's true, it will be an anonymous user and if it's
173 - * false, the user will be loaded from credidentails provided by cookies. This
174 - * object have to be in $wgUser global.
175 - */
176 -class StubUser extends StubObject {
177 -
178 - function __construct() {
179 - parent::__construct( 'wgUser' );
180 - }
181 -
182 - function __call( $name, $args ) {
183 - return $this->_call( $name, $args );
184 - }
185 -
186 - function _newObject() {
187 - global $wgCommandLineMode;
188 - if( $wgCommandLineMode ) {
189 - $user = new User;
190 - } else {
191 - $user = User::newFromSession();
192 - }
193 - return $user;
194 - }
195 -}
Index: trunk/phase3/includes/Setup.php
@@ -322,8 +322,7 @@
323323
324324 // Now that variant lists may be available...
325325 $wgRequest->interpolateTitle();
326 -
327 -$wgUser = new StubUser;
 326+$wgUser = $wgCommandLineMode ? new User : User::newFromSession();
328327 $wgLang = new StubUserLang;
329328 $wgOut = new StubObject( 'wgOut', 'OutputPage' );
330329 $wgParser = new StubObject( 'wgParser', $wgParserConf['class'], array( $wgParserConf ) );
Index: trunk/phase3/includes/AutoLoader.php
@@ -214,7 +214,6 @@
215215 'SquidPurgeClientPool' => 'includes/SquidPurgeClient.php',
216216 'Status' => 'includes/Status.php',
217217 'StubContLang' => 'includes/StubObject.php',
218 - 'StubUser' => 'includes/StubObject.php',
219218 'StubUserLang' => 'includes/StubObject.php',
220219 'StubObject' => 'includes/StubObject.php',
221220 'StringUtils' => 'includes/StringUtils.php',

Follow-up revisions

RevisionCommit summaryAuthorDate
r71029Fix #1 for r70970: Make UploadFromUrlTestSuite initialize a User instead of S...demon14:36, 13 August 2010
r71033Try #2 for fixing r70970 (now with less unrelated changes). Just set $wgUser ...demon15:10, 13 August 2010
r72316Follow-up r70970. There's no StubUser any more, so no point in showing the wf...platonides21:39, 3 September 2010
r74932$wgUser is never a Stub since r70970....platonides22:33, 17 October 2010
r74934$wgUser is never a Stub since r70970.platonides22:47, 17 October 2010

Comments

#Comment by 😂 (talk | contribs)   17:12, 12 August 2010

That was 20µs.

#Comment by Platonides (talk | contribs)   17:39, 12 August 2010
  • UploadFromUrlTestSuite initialises $wgUser to a new StubUser
  • ApiMain sends a debug message if $wgUser isn't stubbed.
#Comment by Platonides (talk | contribs)   17:41, 12 August 2010
  • LiquidThreads also tests for StubUser, but that may be left for now.
#Comment by 😂 (talk | contribs)   14:25, 13 September 2010

Resetting to new, both of these concerns were handled in followups.

#Comment by Aaron Schulz (talk | contribs)   00:46, 21 August 2010

This also removes a type-checking gotcha.

#Comment by Tim Starling (talk | contribs)   01:21, 18 October 2010

The point of StubUser is to avoid parsing loading and parsing User.php, which can be very slow on non-APC installations. It's not about object construction, which is fast either way.

#Comment by Catrope (talk | contribs)   20:26, 7 December 2010

Code looks fine to me, issues resolved, so marking this OK. Tim, I'm leaving the consideration of whether StubUser should be reintroduced or not up to you.

Status & tagging log