jenkins-bot has submitted this change and it was merged.
Change subject: MobilePage: Avoid uncached query for Last modified timestamp
......................................................................
MobilePage: Avoid uncached query for Last modified timestamp
MediaWiki's SkinTemplate contains a variable "lastmod" for this, set
by Skin::lastModified() via OutputPage::getRevisionTimestamp()
which is filled in from ParserOutput cache in Article.php.
According to Xenon, SkinMinerva::getHistoryLink spends most of its
time doing at least 3 database queries, even for simple page views.
SkinMinerva::getHistoryLink:
- Message::parse
- MobilePage::__construct
1. Database::select (via PageImages::getPageImage)
- MobilePage::getLatestTimestamp
2. Database::select (via Revision::getTimestampFromId)
- MobilePage::getLatestEdit
3. Database::select (via Revision::newFromId)
4. Database::select (via MobilePage::getLatestTimestamp)
Changes:
* Avoid #2 by calling OutputPage::getRevisionTimestamp instead, which has
the same value pre-populated from ParserOutput cache.
* Avoid #3 by using Revision::newKnownCurrent, which uses Memcached.
* Avoid #4 in some cases by making MobilePage remember the result of #2
in the instance instead of querying the same thing twice.
Also:
* Remove redundant explicit User::load() call. Fields are lazy-loaded.
It was added in 0a68545711 to avoid master queries, but slave-db has
been the default in User for a while now (mid-2015; dd42294d2).
* Document the non-obvious difference in timestamp format between
getLatestEdit and getLatestTimestamp.
Change-Id: Ia135e61604fa8551caac50dca43b5dc9154babd4
---
M includes/models/MobilePage.php
M includes/skins/SkinMinerva.php
2 files changed, 41 insertions(+), 10 deletions(-)
Approvals:
Gilles: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/models/MobilePage.php b/includes/models/MobilePage.php
index 03e53ac..4f24076 100644
--- a/includes/models/MobilePage.php
+++ b/includes/models/MobilePage.php
@@ -17,6 +17,10 @@
*/
private $title;
/**
+ * @var Revision|bool
+ */
+ private $rev;
+ /**
* @var File Associated page image file (see PageImages extension)
*/
private $file;
@@ -43,32 +47,56 @@
}
}
+ private function getRevision() {
+ if ( $this->rev === null ) {
+ $this->rev = Revision::newKnownCurrent(
+ wfGetDB( DB_REPLICA ),
+ $this->title->getArticleID(),
+ $this->title->getLatestRevID()
+ );
+ }
+ return $this->rev;
+ }
+
/**
- * Retrieve the last time the page content was modified. Do not reflect
null edits.
- * @return string timestamp representing last edited time.
+ * Retrieve timestamp when the page content was last modified. Does not
reflect null edits.
+ * @return string|bool Timestamp (MW format) or false
*/
public function getLatestTimestamp() {
- $title = $this->getTitle();
- return Revision::getTimestampFromId( $title,
$title->getLatestRevID() );
+ if ( $this->revisionTimestamp === null ) {
+ $rev = $this->getRevision();
+ $this->revisionTimestamp = $rev ? $rev->getTimestamp()
: false;
+ }
+ return $this->revisionTimestamp;
+ }
+
+ /**
+ * Set rev_timestamp of latest edit to this page
+ * @param string Timestamp (MW format)
+ */
+ public function setLatestTimestamp( $timestamp ) {
+ $this->revisionTimestamp = $timestamp;
}
/**
* Retrieve the last edit to this page.
- * @return array defining edit with keys name, timestamp and gender
+ * @return array defining edit with keys:
+ * - string name
+ * - string timestamp (Unix format)
+ * - string gender
*/
public function getLatestEdit() {
- $rev = Revision::newFromId( $this->getTitle()->getLatestRevID()
);
- $unixTimestamp = wfTimestamp( TS_UNIX,
$this->getLatestTimestamp() );
+ $rev = $this->getRevision();
$edit = [
- 'timestamp' => $unixTimestamp,
+ 'timestamp' => false,
'name' => '',
'gender' => '',
];
if ( $rev ) {
+ $edit['timestamp'] = wfTimestamp( TS_UNIX,
$rev->getTimestamp() );
$userId = $rev->getUser();
if ( $userId ) {
$revUser = User::newFromId( $userId );
- $revUser->load( User::READ_NORMAL );
$edit['name'] = $revUser->getName();
$edit['gender'] = $revUser->getOption( 'gender'
);
}
diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php
index 9c4c02d..00f2066 100644
--- a/includes/skins/SkinMinerva.php
+++ b/includes/skins/SkinMinerva.php
@@ -690,8 +690,10 @@
protected function getHistoryLink( Title $title ) {
$user = $this->getUser();
$isMainPage = $title->isMainPage();
+ // Get rev_timestamp of current revision (preloaded by
MediaWiki core)
+ $timestamp = $this->getOutput()->getRevisionTimestamp();
$mp = new MobilePage( $this->getTitle(), false );
- $timestamp = $mp->getLatestTimestamp();
+ $mp->setLatestTimestamp( $timestamp );
// Main pages tend to include transclusions (see bug 51924)
if ( $isMainPage ) {
$lastModified = $this->msg( 'mobile-frontend-history'
)->plain();
@@ -704,6 +706,7 @@
}
$edit = $mp->getLatestEdit();
$link = [
+ // Use $edit['timestamp'] (Unix format) instead of
$timestamp (MW format)
'data-timestamp' => $isMainPage ? '' :
$edit['timestamp'],
'href' => SpecialPage::getTitleFor( 'History', $title
)->getLocalURL(),
'text' => $lastModified,
--
To view, visit https://gerrit.wikimedia.org/r/314787
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia135e61604fa8551caac50dca43b5dc9154babd4
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Gilles <gdu...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Ori.livneh <o...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
