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 <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Demon <[email protected]>
Gerrit-Reviewer: Tim Starling <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
