Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache self.ToLuaValue in ScriptTriggers #9978

Merged
merged 1 commit into from Nov 22, 2015

Conversation

RoosterDragon
Copy link
Member

(This explanation is probably going to be terrible, so tl;dr: This improves performance.)

When making an Lua function call, any LuaCustomClrObject must be introspected via reflection in order to determine what to expose in Lua code. In OpenRA, we use these for any types that implement IScriptBindable, such as Actor.

Previously, we would need to pay the cost of this reflection for every individual Lua call an Actor used in its ScriptTriggers trait where it passed self as a parameter. This would be repeated every time. For performance, we now cache self.ToLuaValue in the trait and use that for all calls so we only pay the reflection cost once on trait construction. This removes a significant overhead in the Lua bridging code.

This requires OpenRA/Eluant#1 in order to realize the performance benefits - but this PR doesn't strictly depend on it and can be merged on its own merits as desired.

Alternatively, in pseudocode:

Calling an Lua function and passing an actor looks like this:

void DoSomeLua()
{
    using (var a = actor.ToLuaValue(context))
        luaFunction.Call(a);
}

This is morally equivalent to:

void DoSomeLua()
{
    using (var a = actor.ToLuaValue(context))
        luaFunction.Call(a.ExpensiveReflectionHere());
}

The linked PR changes this to:

void DoSomeLua()
{
    using (var a = actor.ToLuaValue(context).ExpensiveReflectionHere())
        luaFunction.Call(a);
}

This PR then changes this to:

// This is a field which we reuse, so we don't have to pay for reflection every time.
LuaValue a = actor.ToLuaValue(context).ExpensiveReflectionHere();

void DoSomeLua()
{
    luaFunction.Call(a);
}

On the RA shellmap, this reduces the reflection overhead in calling Lua functions from 4.9% of CPU to 0.9%. This is mainly due to ScriptTriggers.TickIdle since that passes an actor parameter.

When making an Lua function call, any LuaCustomClrObject must be introspected via reflection in order to determine what to expose in Lua code. In OpenRA, we use these for any types that implement IScriptBindable, such as Actor.

Previously, we would need to pay the cost of this reflection for every individual Lua call an Actor used in its ScriptTriggers trait where it passed `self` as a parameter. This would be repeated every time. For performance, we now cache self.ToLuaValue in the trait and use that for all calls so we only pay the reflection cost once on trait construction. This removes a significant overhead in the Lua bridging code.
@penev92
Copy link
Member

penev92 commented Nov 13, 2015

Code looks good :+0.5:

Also the explanation is pretty good. The example code at the end is pretty clear ;)

@penev92
Copy link
Member

penev92 commented Nov 14, 2015

👍

@Mailaender
Copy link
Member

Scripts still trigger stuff. ✅

Mailaender added a commit that referenced this pull request Nov 22, 2015
@Mailaender Mailaender merged commit 79bf69c into OpenRA:bleed Nov 22, 2015
@Mailaender
Copy link
Member

Changelog

@RoosterDragon RoosterDragon deleted the cache-script-actor-luavalue branch November 22, 2015 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants