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