{"action":"created","issue":{"url":"https://api.github.com/repos/real-logic/simple-binary-encoding/issues/588","repository_url":"https://api.github.com/repos/real-logic/simple-binary-encoding","labels_url":"https://api.github.com/repos/real-logic/simple-binary-encoding/issues/588/labels{/name}","comments_url":"https://api.github.com/repos/real-logic/simple-binary-encoding/issues/588/comments","events_url":"https://api.github.com/repos/real-logic/simple-binary-encoding/issues/588/events","html_url":"https://github.com/real-logic/simple-binary-encoding/issues/588","id":365222549,"node_id":"MDU6SXNzdWUzNjUyMjI1NDk=","number":588,"title":"[C#] Use throw helpers for generated code","user":{"login":"damageboy","id":125730,"node_id":"MDQ6VXNlcjEyNTczMA==","avatar_url":"https://avatars3.githubusercontent.com/u/125730?v=4","gravatar_id":"","url":"https://api.github.com/users/damageboy","html_url":"https://github.com/damageboy","followers_url":"https://api.github.com/users/damageboy/followers","following_url":"https://api.github.com/users/damageboy/following{/other_user}","gists_url":"https://api.github.com/users/damageboy/gists{/gist_id}","starred_url":"https://api.github.com/users/damageboy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/damageboy/subscriptions","organizations_url":"https://api.github.com/users/damageboy/orgs","repos_url":"https://api.github.com/users/damageboy/repos","events_url":"https://api.github.com/users/damageboy/events{/privacy}","received_events_url":"https://api.github.com/users/damageboy/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-09-30T12:54:24Z","updated_at":"2018-10-11T10:06:38Z","closed_at":null,"author_association":"NONE","body":"Currently the generated code in the C# generated Getters/Setters for fixed array types and a few other cases calls a rather lengthy set of operations for the uncommon path of throwing an exception in case the provided parameters were not constructed properly.\r\n\r\nWhile this is of course important, for the general case, this is also somewhat inefficient in its current form as the code to generate the exception is being repeated many times, leading to generated code bloat, esp. when it comes down to the JITed code size.\r\n\r\nI'm copy-pasting this from CoreCLR's own [`ThrowHelper.cs`](https://github.com/dotnet/coreclr/blob/master/src/System.Private.CoreLib/src/System/ThrowHelper.cs#L6-L36):\r\n\r\n```csharp\r\n// This file defines an internal class used to throw exceptions in BCL code.\r\n// The main purpose is to reduce code size. \r\n// \r\n// The old way to throw an exception generates quite a lot IL code and assembly code.\r\n// Following is an example:\r\n//     C# source\r\n//          throw new ArgumentNullException(nameof(key), SR.ArgumentNull_Key);\r\n//     IL code:\r\n//          IL_0003:  ldstr      \"key\"\r\n//          IL_0008:  ldstr      \"ArgumentNull_Key\"\r\n//          IL_000d:  call       string System.Environment::GetResourceString(string)\r\n//          IL_0012:  newobj     instance void System.ArgumentNullException::.ctor(string,string)\r\n//          IL_0017:  throw\r\n//    which is 21bytes in IL.\r\n// \r\n// So we want to get rid of the ldstr and call to Environment.GetResource in IL.\r\n// In order to do that, I created two enums: ExceptionResource, ExceptionArgument to represent the\r\n// argument name and resource name in a small integer. The source code will be changed to \r\n//    ThrowHelper.ThrowArgumentNullException(ExceptionArgument.key, ExceptionResource.ArgumentNull_Key);\r\n//\r\n// The IL code will be 7 bytes.\r\n//    IL_0008:  ldc.i4.4\r\n//    IL_0009:  ldc.i4.4\r\n//    IL_000a:  call       void System.ThrowHelper::ThrowArgumentNullException(valuetype System.ExceptionArgument)\r\n//    IL_000f:  ldarg.0\r\n//\r\n// This will also reduce the Jitted code size a lot. \r\n//\r\n// It is very important we do this for generic classes because we can easily generate the same code \r\n// multiple times for different instantiation. \r\n// \r\n```\r\n\r\nThis is even more severe in the generated SBE code, as due to string formatting each such block is 27 bytes long rather than the 21 mentioned in CoreCLR for their \"pattern\".\r\n\r\nI would like to work on adding a minimal ThrowHelper.cs to `SBE.dll` and do the work of calling the throw helpers rather than generated the current exception throwing code, Any thoughts or comments about this?\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/real-logic/simple-binary-encoding/issues/comments/428898354","html_url":"https://github.com/real-logic/simple-binary-encoding/issues/588#issuecomment-428898354","issue_url":"https://api.github.com/repos/real-logic/simple-binary-encoding/issues/588","id":428898354,"node_id":"MDEyOklzc3VlQ29tbWVudDQyODg5ODM1NA==","user":{"login":"damageboy","id":125730,"node_id":"MDQ6VXNlcjEyNTczMA==","avatar_url":"https://avatars3.githubusercontent.com/u/125730?v=4","gravatar_id":"","url":"https://api.github.com/users/damageboy","html_url":"https://github.com/damageboy","followers_url":"https://api.github.com/users/damageboy/followers","following_url":"https://api.github.com/users/damageboy/following{/other_user}","gists_url":"https://api.github.com/users/damageboy/gists{/gist_id}","starred_url":"https://api.github.com/users/damageboy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/damageboy/subscriptions","organizations_url":"https://api.github.com/users/damageboy/orgs","repos_url":"https://api.github.com/users/damageboy/repos","events_url":"https://api.github.com/users/damageboy/events{/privacy}","received_events_url":"https://api.github.com/users/damageboy/received_events","type":"User","site_admin":false},"created_at":"2018-10-11T10:06:38Z","updated_at":"2018-10-11T10:06:38Z","author_association":"NONE","body":"@ZachBray Yes, another side-effect of this is that this potentially makes some/all getters inlineable by the JIT by default without needing to force inlining with things such as `AggressiveInlining`.\r\n\r\n"}}
{"actor":{"display_login":"damageboy"}}