Timing::measure(): handle missing marks better

Currently Timing::measure() does not check that the requested start and end
marks exist, causing it to return bogus values without any indication that
something has gone wrong. Fix this by logging and error and returning false
in case either the start or end markers do not exist. To make it possible to
log, make Timing implement the LoggerAware interface.

Change-Id: I75af5273e9a8a52b31d0af1de206b0d8a4c82fbc

diff --git a/includes/context/RequestContext.php b/includes/context/RequestContext.php
index 4f8e65d..a0073aa 100644 (file)
--- a/includes/context/RequestContext.php
+++ b/includes/context/RequestContext.php
@@ -22,6 +22,8 @@
  * @file
  */
 
+use MediaWiki\Logger\LoggerFactory;
+
 /**
  * Group all the pieces relevant to the context of a request into one instance
  */
@@ -151,7 +153,9 @@ class RequestContext implements IContextSource, MutableContext {
         */
        public function getTiming() {
                if ( $this->timing === null ) {
-                       $this->timing = new Timing();
+                       $this->timing = new Timing( array(
+                               'logger' => LoggerFactory::getInstance( 'Timing' )
+                       ) );
                }
                return $this->timing;
        }

diff --git a/includes/libs/Timing.php b/includes/libs/Timing.php
index 653227e..00ceda3 100644 (file)
--- a/includes/libs/Timing.php
+++ b/includes/libs/Timing.php
@@ -18,6 +18,10 @@
  * @file
  */
 
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+
 /**
  * An interface to help developers measure the performance of their applications.
  * This interface closely matches the W3C's User Timing specification.
@@ -38,13 +42,27 @@
  *
  * @since 1.27
  */
-class Timing {
+class Timing implements LoggerAwareInterface {
 
        /** @var array[] */
        private $entries = array();
 
-       public function __construct() {
+       /** @var LoggerInterface */
+       protected $logger;
+
+       public function __construct( array $params = array() ) {
                $this->clearMarks();
+               $this->setLogger( isset( $params['logger'] ) ? $params['logger'] : new NullLogger() );
+       }
+
+       /**
+        * Sets a logger instance on the object.
+        *
+        * @param LoggerInterface $logger
+        * @return null
+        */
+       public function setLogger( LoggerInterface $logger ) {
+               $this->logger = $logger;
        }
 
        /**
@@ -102,14 +120,23 @@ class Timing {
         * @param string $measureName
         * @param string $startMark
         * @param string $endMark
-        * @return array The measure that has been created.
+        * @return array|bool The measure that has been created, or false if either
+        *  the start mark or the end mark do not exist.
         */
        public function measure( $measureName, $startMark = 'requestStart', $endMark = null ) {
                $start = $this->getEntryByName( $startMark );
+               if ( $start === null ) {
+                       $this->logger->error( __METHOD__ . ": The mark '$startMark' does not exist" );
+                       return false;
+               }
                $startTime = $start['startTime'];
 
                if ( $endMark ) {
                        $end = $this->getEntryByName( $endMark );
+                       if ( $end === null ) {
+                               $this->logger->error( __METHOD__ . ": The mark '$endMark' does not exist" );
+                               return false;
+                       }
                        $endTime = $end['startTime'];
                } else {
                        $endTime = microtime( true );