Page MenuHomePhabricator

If possible, turn off $wgAutoloadAttemptLowercase
Closed, ResolvedPublic

Description

perf on a random app server showed >1.4% of all cycles being spent in array_change_key_case() in AutoLoader.php#59. This code is only hit if $wgAutoloadAttemptLowercase is true. Is it still needed?

Capturing messages on the autoloader debug log channel could help determining that. If it's not needed, setting this config var to false would be a easy modest performance win.

Event Timeline

ori created this task.Jun 1 2017, 6:11 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 1 2017, 6:11 AM
Krinkle edited subscribers, added: Krinkle; removed: Performance-Team.
ori added a comment.Jun 2 2017, 2:35 PM

Since this code is hot, I assume AutoLoader.php runs before the Composer autoloader, and that the case manipulation happens for every class that lives in an external depedency. If that is true, then it might be good idea to relegate the code that does the case manipulation to a discrete legacy autoloader that runs last.

Change 356841 had a related patch set uploaded (by Krinkle; owner: Ori Livneh):
[operations/mediawiki-config@master] Capture messages on 'autoloader' debug log channel

https://gerrit.wikimedia.org/r/356841

Gilles assigned this task to aaron.Jun 7 2017, 7:38 PM
Gilles moved this task from Inbox to Doing on the Performance-Team board.
Krinkle triaged this task as Medium priority.Jun 7 2017, 7:38 PM
Legoktm added a subscriber: Legoktm.Jun 7 2017, 8:08 PM

We run the MediaWiki autoloader before composer because at that time most classes were still in MediaWiki's autoloader - we should check that it's still the case. IIRC there are PHP4 serialized objects in the database somewhere that depend upon being able to autoload with the lowercase class name.

Change 356841 merged by jenkins-bot:
[operations/mediawiki-config@master] Capture messages on 'autoloader' debug log channel

https://gerrit.wikimedia.org/r/356841

aaron added a comment.Jun 14 2017, 7:13 PM

Log entries show for:

Class Mediawiki\Widget\DateInputWidget was loaded using incorrect case	34,321
Class SpecialRecentchanges was loaded using incorrect case	7,867
Class SpecialRecentchangeslinked was loaded using incorrect case	6,618
Class historyblobcurstub was loaded using incorrect case	3,249
Class xml was loaded using incorrect case	26
Class concatenatedgziphistoryblob was loaded using incorrect case	14
Class historyblobstub was loaded using incorrect case	7

Class Mediawiki\Widget\DateInputWidget was loaded using incorrect case 34,321

Fixed in https://gerrit.wikimedia.org/r/361095

Class SpecialRecentchanges was loaded using incorrect case 7,867
Class SpecialRecentchangeslinked was loaded using incorrect case 6,618

https://gerrit.wikimedia.org/r/361096

All the other ones (except xml?) look like they're serialized stuff from the PHP4 era.

history blobs are tricky since they are probably serialized in the older read-only DBs that we don't normally mess around with (some are just clones with no replication AFAIK).

If we have a small list of classes that we know that could be serialized, could we just add their lowercase variants to the autoload classmap?

aaron added a comment.Jun 24 2017, 4:34 AM

Seems like the most viable option.

aaron removed aaron as the assignee of this task.Jul 5 2017, 11:27 PM
aaron added a subscriber: aaron.
Krinkle moved this task from Doing to Radar on the Performance-Team board.Jul 12 2017, 4:36 AM
matmarex removed a subscriber: matmarex.Sep 11 2017, 3:10 PM
ori added a comment.Oct 12 2017, 8:44 PM

Log entries show for:

Class Mediawiki\Widget\DateInputWidget was loaded using incorrect case	34,321
Class SpecialRecentchanges was loaded using incorrect case	7,867
Class SpecialRecentchangeslinked was loaded using incorrect case	6,618
Class historyblobcurstub was loaded using incorrect case	3,249
Class xml was loaded using incorrect case	26
Class concatenatedgziphistoryblob was loaded using incorrect case	14
Class historyblobstub was loaded using incorrect case	7

How about now?

Krinkle added a comment.EditedOct 12 2017, 10:06 PM

How about now?

$ head -n1 autoloader.log && tail -n1 autoloader.log 
2017-10-12 07:41:03 ..
2017-10-12 22:05:42 ..

$ cat autoloader.log  | cut -d' ' -f9- | sort | uniq -c
   1379 Class concatenatedgziphistoryblob was loaded using incorrect case  
  33433 Class historyblobcurstub was loaded using incorrect case  
    247 Class historyblobstub was loaded using incorrect case  
     26 Class HtmlForm was loaded using incorrect case  
     82 Class HTML was loaded using incorrect case  
      5 Class HTTP was loaded using incorrect case  
    361 Class Mediawiki\Widget\SelectWithInputWidget was loaded using incorrect case  
     97 Class XML was loaded using incorrect case

Change 384288 had a related patch set uploaded (by Ori.livneh; owner: Ori Livneh):
[mediawiki/core@master] Add lowercase variants to the autoloader for legacy history blob classes

https://gerrit.wikimedia.org/r/384288

Change 384303 had a related patch set uploaded (by Ori.livneh; owner: Ori Livneh):
[mediawiki/extensions/Wikibase@master] Fix letter-case of XML

https://gerrit.wikimedia.org/r/384303

Change 384308 had a related patch set uploaded (by Ori.livneh; owner: Ori Livneh):
[mediawiki/extensions/PageAssessments@master] Fix class name letter-case

https://gerrit.wikimedia.org/r/384308

Change 384309 had a related patch set uploaded (by Ori.livneh; owner: Ori Livneh):
[mediawiki/extensions/UploadWizard@master] Fix letter-case of class name

https://gerrit.wikimedia.org/r/384309

Change 384310 had a related patch set uploaded (by Ori.livneh; owner: Ori Livneh):
[mediawiki/extensions/MassMessage@master] Fix letter-case of class name

https://gerrit.wikimedia.org/r/384310

Change 384308 merged by jenkins-bot:
[mediawiki/extensions/PageAssessments@master] Fix class name letter-case

https://gerrit.wikimedia.org/r/384308

Change 384309 merged by jenkins-bot:
[mediawiki/extensions/UploadWizard@master] Fix letter-case of class name

https://gerrit.wikimedia.org/r/384309

Change 384310 merged by jenkins-bot:
[mediawiki/extensions/MassMessage@master] Fix letter-case of class name

https://gerrit.wikimedia.org/r/384310

Change 384303 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Fix letter-case of XML

https://gerrit.wikimedia.org/r/384303

Change 384288 merged by jenkins-bot:
[mediawiki/core@master] Add lowercase variants to the autoloader for legacy history blob classes

https://gerrit.wikimedia.org/r/384288

Change 385405 had a related patch set uploaded (by Krinkle; owner: Ori Livneh):
[integration/jenkins@master] Add $wgAutoloadAttemptLowercase = false to dev settings

https://gerrit.wikimedia.org/r/385405

Change 385405 merged by jenkins-bot:
[integration/jenkins@master] Add $wgAutoloadAttemptLowercase = false to dev settings

https://gerrit.wikimedia.org/r/385405

Change 385978 had a related patch set uploaded (by Ori.livneh; owner: Ori Livneh):
[operations/mediawiki-config@master] labs: Set $wgAutoloadAttemptLowercase to false

https://gerrit.wikimedia.org/r/385978

Change 385978 merged by jenkins-bot:
[operations/mediawiki-config@master] labs: Set $wgAutoloadAttemptLowercase to false

https://gerrit.wikimedia.org/r/385978

ori added a comment.Oct 23 2017, 2:25 PM

There is only a single entry in autoloader.log for the past three days:

2017-10-23 13:11:02 [We3qZgpAEDQAAJt3Qz0AAABF] mw1287 wikidatawiki 1.31.0-wmf.4 autoloader INFO: Class HTTP was loaded using incorrect case

I can't find the culprit code, but it is not possible to grep for certain patterns (e.g. $cls = 'HTTP'; $req = new $cls;).

$wgAutoloadAttemptLowercase is currently set to false for Jenkins tests, MediaWiki-Vagrant, and Labs.

Change 386384 had a related patch set uploaded (by Ori.livneh; owner: Ori Livneh):
[operations/mediawiki-config@master] Set $wgAutoloadAttemptLowercase to false

https://gerrit.wikimedia.org/r/386384

Change 386384 merged by jenkins-bot:
[operations/mediawiki-config@master] Set $wgAutoloadAttemptLowercase to false

https://gerrit.wikimedia.org/r/386384

Mentioned in SAL (#wikimedia-operations) [2017-10-25T14:47:09Z] <mattflaschen@tin> Synchronized wmf-config/InitialiseSettings.php: T166759: Set $wgAutoloadAttemptLowercase to false (duration: 00m 50s)

Krinkle assigned this task to ori.Oct 25 2017, 3:41 PM
Krinkle moved this task from Radar to Doing on the Performance-Team board.
Krinkle edited projects, added Performance-Team; removed Performance-Team (Radar).
Krinkle removed a project: Patch-For-Review.
Krinkle closed this task as Resolved.Nov 1 2017, 1:52 AM

Change 387971 abandoned by Krinkle:
Re-enable $wgAutoloadAttemptLowercase for older branches

https://gerrit.wikimedia.org/r/387971

Change 398453 had a related patch set uploaded (by Mainframe98; owner: Mainframe98):
[mediawiki/core@master] Fix additional usage of incorrect case

https://gerrit.wikimedia.org/r/398453

Change 398453 merged by jenkins-bot:
[mediawiki/core@master] Fix additional usage of incorrect case

https://gerrit.wikimedia.org/r/398453

Change 415759 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[operations/mediawiki-config@master] beta: remove $wgAutoloadAttemptLowercase

https://gerrit.wikimedia.org/r/415759

Change 415759 merged by jenkins-bot:
[operations/mediawiki-config@master] beta: remove $wgAutoloadAttemptLowercase

https://gerrit.wikimedia.org/r/415759