{"action":"created","issue":{"url":"https://api.github.com/repos/OpenRA/OpenRA/issues/9978","labels_url":"https://api.github.com/repos/OpenRA/OpenRA/issues/9978/labels{/name}","comments_url":"https://api.github.com/repos/OpenRA/OpenRA/issues/9978/comments","events_url":"https://api.github.com/repos/OpenRA/OpenRA/issues/9978/events","html_url":"https://github.com/OpenRA/OpenRA/pull/9978","id":116666357,"number":9978,"title":"Cache self.ToLuaValue in ScriptTriggers","user":{"login":"RoosterDragon","id":3399086,"avatar_url":"https://avatars.githubusercontent.com/u/3399086?v=3","gravatar_id":"","url":"https://api.github.com/users/RoosterDragon","html_url":"https://github.com/RoosterDragon","followers_url":"https://api.github.com/users/RoosterDragon/followers","following_url":"https://api.github.com/users/RoosterDragon/following{/other_user}","gists_url":"https://api.github.com/users/RoosterDragon/gists{/gist_id}","starred_url":"https://api.github.com/users/RoosterDragon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RoosterDragon/subscriptions","organizations_url":"https://api.github.com/users/RoosterDragon/orgs","repos_url":"https://api.github.com/users/RoosterDragon/repos","events_url":"https://api.github.com/users/RoosterDragon/events{/privacy}","received_events_url":"https://api.github.com/users/RoosterDragon/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/OpenRA/OpenRA/labels/PR:%20Needs%20+2","name":"PR: Needs +2","color":"009800"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-11-12T23:33:41Z","updated_at":"2015-11-22T17:06:32Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/OpenRA/OpenRA/pulls/9978","html_url":"https://github.com/OpenRA/OpenRA/pull/9978","diff_url":"https://github.com/OpenRA/OpenRA/pull/9978.diff","patch_url":"https://github.com/OpenRA/OpenRA/pull/9978.patch"},"body":"(This explanation is probably going to be terrible, so tl;dr: This improves performance.)\r\n\r\nWhen 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.\r\n\r\nPreviously, 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.\r\n\r\nThis 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.\r\n\r\nAlternatively, in pseudocode:\r\n\r\nCalling an Lua function and passing an actor looks like this:\r\n```csharp\r\nvoid DoSomeLua()\r\n{\r\n    using (var a = actor.ToLuaValue(context))\r\n        luaFunction.Call(a);\r\n}\r\n```\r\n\r\nThis is morally equivalent to:\r\n```csharp\r\nvoid DoSomeLua()\r\n{\r\n    using (var a = actor.ToLuaValue(context))\r\n        luaFunction.Call(a.ExpensiveReflectionHere());\r\n}\r\n```\r\n\r\nThe linked PR changes this to:\r\n```csharp\r\nvoid DoSomeLua()\r\n{\r\n    using (var a = actor.ToLuaValue(context).ExpensiveReflectionHere())\r\n        luaFunction.Call(a);\r\n}\r\n```\r\n\r\nThis PR then changes this to:\r\n```csharp\r\n// This is a field which we reuse, so we don't have to pay for reflection every time.\r\nLuaValue a = actor.ToLuaValue(context).ExpensiveReflectionHere();\r\n\r\nvoid DoSomeLua()\r\n{\r\n    luaFunction.Call(a);\r\n}\r\n```\r\n\r\nOn 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."},"comment":{"url":"https://api.github.com/repos/OpenRA/OpenRA/issues/comments/158778618","html_url":"https://github.com/OpenRA/OpenRA/pull/9978#issuecomment-158778618","issue_url":"https://api.github.com/repos/OpenRA/OpenRA/issues/9978","id":158778618,"user":{"login":"Mailaender","id":756669,"avatar_url":"https://avatars.githubusercontent.com/u/756669?v=3","gravatar_id":"","url":"https://api.github.com/users/Mailaender","html_url":"https://github.com/Mailaender","followers_url":"https://api.github.com/users/Mailaender/followers","following_url":"https://api.github.com/users/Mailaender/following{/other_user}","gists_url":"https://api.github.com/users/Mailaender/gists{/gist_id}","starred_url":"https://api.github.com/users/Mailaender/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Mailaender/subscriptions","organizations_url":"https://api.github.com/users/Mailaender/orgs","repos_url":"https://api.github.com/users/Mailaender/repos","events_url":"https://api.github.com/users/Mailaender/events{/privacy}","received_events_url":"https://api.github.com/users/Mailaender/received_events","type":"User","site_admin":false},"created_at":"2015-11-22T17:06:32Z","updated_at":"2015-11-22T17:06:32Z","body":"Scripts still trigger stuff. :white_check_mark: "}}