Tim Starling has submitted this change and it was merged.

Change subject: (bug 45684) Don't count argument parsing time towards Lua limit
......................................................................


(bug 45684) Don't count argument parsing time towards Lua limit

Currently, the time taken to parse the arguments passed to a Lua
function from #invoke will be counted against Lua's 10-second limit.
This is counterintuitive, and can remove incentive for users to convert
templates to Lua since they may have to convert a whole stack at once.

Note this requires change I11881232 to mediawiki/php/luasandbox to
actually have any effect.

Bug: 45684
Change-Id: I773950e4c399b8a1cfa6d1cde781a069d286b3bd
---
M engines/LuaCommon/LuaCommon.php
M engines/LuaCommon/LuaInterpreter.php
M engines/LuaSandbox/Engine.php
M engines/LuaStandalone/LuaStandaloneEngine.php
M tests/engines/LuaSandbox/SandboxTest.php
5 files changed, 102 insertions(+), 1 deletion(-)

Approvals:
  Tim Starling: Verified; Looks good to me, approved



diff --git a/engines/LuaCommon/LuaCommon.php b/engines/LuaCommon/LuaCommon.php
index cbf15d2..e38abf8 100644
--- a/engines/LuaCommon/LuaCommon.php
+++ b/engines/LuaCommon/LuaCommon.php
@@ -363,6 +363,7 @@
                if ( $frame === false ) {
                        return array();
                }
+               $this->getInterpreter()->pauseUsageTimer();
                $result = $frame->getArgument( $name );
                if ( $result === false ) {
                        return array();
@@ -379,6 +380,7 @@
                if ( $frame === false ) {
                        return array();
                }
+               $this->getInterpreter()->pauseUsageTimer();
                return array( $frame->getArguments() );
        }
 
@@ -432,10 +434,17 @@
                if ( !$frame ) {
                        throw new Scribunto_LuaError( 'attempt to call 
mw.preprocess with no frame' );
                }
+
+               // Don't count the time for expanding all the frame arguments 
against
+               // the Lua time limit.
+               $this->getInterpreter()->pauseUsageTimer();
+               $args = $frame->getArguments();
+               $this->getInterpreter()->unpauseUsageTimer();
+
                $text = $this->doCachedExpansion( $frame, $text,
                        array(
                                'inputText' => $text,
-                               'args' => $frame->getArguments()
+                               'args' => $args,
                        ) );
                return array( $text );
        }
diff --git a/engines/LuaCommon/LuaInterpreter.php 
b/engines/LuaCommon/LuaInterpreter.php
index 476b806..9567ff9 100644
--- a/engines/LuaCommon/LuaInterpreter.php
+++ b/engines/LuaCommon/LuaInterpreter.php
@@ -45,6 +45,18 @@
         *    pcall(). 
         */
        abstract public function registerLibrary( $name, $functions );
+
+       /**
+        * Pause CPU usage and limits
+        * @return void
+        */
+       abstract public function pauseUsageTimer();
+
+       /**
+        * Unpause CPU usage and limits
+        * @return void
+        */
+       abstract public function unpauseUsageTimer();
 }
 
 class Scribunto_LuaInterpreterNotFoundError extends MWException {}
diff --git a/engines/LuaSandbox/Engine.php b/engines/LuaSandbox/Engine.php
index ee5cc6e..1f957db 100644
--- a/engines/LuaSandbox/Engine.php
+++ b/engines/LuaSandbox/Engine.php
@@ -191,6 +191,18 @@
                        return array();
                }
        }
+
+       public function pauseUsageTimer() {
+               if ( is_callable( array( $this->sandbox, 'pauseUsageTimer' ) ) 
) {
+                       $this->sandbox->pauseUsageTimer();
+               }
+       }
+
+       public function unpauseUsageTimer() {
+               if ( is_callable( array( $this->sandbox, 'unpauseUsageTimer' ) 
) ) {
+                       $this->sandbox->unpauseUsageTimer();
+               }
+       }
 }
 
 class Scribunto_LuaSandboxCallback {
diff --git a/engines/LuaStandalone/LuaStandaloneEngine.php 
b/engines/LuaStandalone/LuaStandaloneEngine.php
index 1970322..f656111 100644
--- a/engines/LuaStandalone/LuaStandaloneEngine.php
+++ b/engines/LuaStandalone/LuaStandaloneEngine.php
@@ -219,6 +219,12 @@
                return $result[1];
        }
 
+       public function pauseUsageTimer() {
+       }
+
+       public function unpauseUsageTimer() {
+       }
+
        /**
         * Fill in missing nulls in a list received from Lua
         *
diff --git a/tests/engines/LuaSandbox/SandboxTest.php 
b/tests/engines/LuaSandbox/SandboxTest.php
index 9ef1e21..d1bc5da 100644
--- a/tests/engines/LuaSandbox/SandboxTest.php
+++ b/tests/engines/LuaSandbox/SandboxTest.php
@@ -12,4 +12,66 @@
                        'SandboxTests' => __DIR__ . '/SandboxTests.lua',
                );
        }
+
+       function testArgumentParsingTime() {
+               $engine = $this->getEngine();
+               if ( !is_callable( array( $engine->getInterpreter()->sandbox, 
'pauseUsageTimer' ) ) ) {
+                       $this->markTestSkipped( "LuaSandbox::pauseUsageTimer is 
not available" );
+               }
+
+               $parser = $engine->getParser();
+               $pp = $parser->getPreprocessor();
+               $frame = $pp->newFrame();
+
+               $parser->setHook( 'scribuntodelay', function () {
+                       $t = microtime( 1 ) + 0.5;
+                       while ( microtime( 1 ) < $t ) {
+                               # Waste CPU cycles
+                       }
+                       return "ok";
+               } );
+               $this->extraModules['Module:TestArgumentParsingTime'] = '
+                       return {
+                               f = function ( frame )
+                                       return frame.args[1]
+                               end,
+                               f2 = function ( frame )
+                                       return frame:preprocess( 
"{{#invoke:TestArgumentParsingTime|f|}}" )
+                               end,
+                               f3 = function ( frame )
+                                       return frame:preprocess( 
"{{#invoke:TestArgumentParsingTime|f|<scribuntodelay/>}}" )
+                               end,
+                       }
+               ';
+
+               $u0 = $engine->getInterpreter()->getCPUUsage();
+               $frame->expand(
+                       $pp->preprocessToObj(
+                               
'{{#invoke:TestArgumentParsingTime|f|<scribuntodelay/>}}'
+                       )
+               );
+               $this->assertLessThan( 0.25, 
$engine->getInterpreter()->getCPUUsage() - $u0,
+                       'Argument access time was not counted'
+               );
+
+               $u0 = $engine->getInterpreter()->getCPUUsage();
+               $frame->expand(
+                       $pp->preprocessToObj(
+                               
'{{#invoke:TestArgumentParsingTime|f2|<scribuntodelay/>}}'
+                       )
+               );
+               $this->assertLessThan( 0.25, 
$engine->getInterpreter()->getCPUUsage() - $u0,
+                       'Unused arguments not counted in preprocess'
+               );
+
+               $u0 = $engine->getInterpreter()->getCPUUsage();
+               $frame->expand(
+                       $pp->preprocessToObj(
+                               '{{#invoke:TestArgumentParsingTime|f3}}'
+                       )
+               );
+               $this->assertGreaterThan( 0.25, 
$engine->getInterpreter()->getCPUUsage() - $u0,
+                       'Recursive argument access time was counted'
+               );
+       }
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/52572
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I773950e4c399b8a1cfa6d1cde781a069d286b3bd
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/Scribunto
Gerrit-Branch: master
Gerrit-Owner: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Anomie <bjor...@wikimedia.org>
Gerrit-Reviewer: Demon <ch...@wikimedia.org>
Gerrit-Reviewer: Tim Starling <tstarl...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to