{"action":"created","issue":{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-dotnet-client/issues/452","repository_url":"https://api.github.com/repos/rabbitmq/rabbitmq-dotnet-client","labels_url":"https://api.github.com/repos/rabbitmq/rabbitmq-dotnet-client/issues/452/labels{/name}","comments_url":"https://api.github.com/repos/rabbitmq/rabbitmq-dotnet-client/issues/452/comments","events_url":"https://api.github.com/repos/rabbitmq/rabbitmq-dotnet-client/issues/452/events","html_url":"https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/452","id":359787792,"node_id":"MDExOlB1bGxSZXF1ZXN0MjE1MTk4MzU0","number":452,"title":"[WIP] Simple version of PooledMemoryStream","user":{"login":"Pliner","id":664889,"node_id":"MDQ6VXNlcjY2NDg4OQ==","avatar_url":"https://avatars3.githubusercontent.com/u/664889?v=4","gravatar_id":"","url":"https://api.github.com/users/Pliner","html_url":"https://github.com/Pliner","followers_url":"https://api.github.com/users/Pliner/followers","following_url":"https://api.github.com/users/Pliner/following{/other_user}","gists_url":"https://api.github.com/users/Pliner/gists{/gist_id}","starred_url":"https://api.github.com/users/Pliner/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Pliner/subscriptions","organizations_url":"https://api.github.com/users/Pliner/orgs","repos_url":"https://api.github.com/users/Pliner/repos","events_url":"https://api.github.com/users/Pliner/events{/privacy}","received_events_url":"https://api.github.com/users/Pliner/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2018-09-13T08:11:18Z","updated_at":"2018-09-17T21:46:37Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-dotnet-client/pulls/452","html_url":"https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/452","diff_url":"https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/452.diff","patch_url":"https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/452.patch"},"body":"## Proposed Changes\r\n\r\nThis PR continues work on allocations(see #443). Its goal is to provide the pool of `MemoryStream` to reduce allocations, which were caused by resize of the internal buffer of `MemoryStream`.\r\n\r\nThe idea of PR is not to pool of `MemoryStream`, but to pool internal buffers. So, `PooledMemoryStream` is introduced, which is the same as `MemoryStream`, but it also pools internal buffers with help of [System.Buffers](https://www.nuget.org/packages/System.Buffers/).\r\n\r\nA bit more info about `System.Buffers`:\r\n1. It has default shared implementation(`ArrayPool<T>.Shared`), which can be reused not only by rabbitmq client.\r\n2. The default implementation has the following allocation strategy: it allocates arrays of size 2^i, where i from 4 to 20. If the size of renting array is larger than 2^20, it will allocate an array outside of the pool.\r\n3. Also it has some trimming strategy, which helps to free memory in case of low memory.\r\n4. This package is referenced transitively by MongoDB Client([DnsClient](https://www.nuget.org/packages/DnsClient/) references `System.Buffers`), so it may be not so awfull to have this package as dependency.\r\n\r\n## Types of Changes\r\n\r\nWhat types of changes does your code introduce to this project?\r\n_Put an `x` in the boxes that apply_\r\n\r\n- [ ] Bugfix (non-breaking change which fixes issue #NNNN)\r\n- [ ] New feature (non-breaking change which adds functionality)\r\n- [X] Breaking change (fix or feature that would cause existing functionality to not work as expected)\r\n- [ ] Documentation (correction or otherwise)\r\n- [ ] Cosmetics (whitespace, appearance)\r\n\r\n## Checklist\r\n\r\n_Put an `x` in the boxes that apply. You can also fill these out after creating\r\nthe PR. If you're unsure about any of them, don't hesitate to ask on the\r\nmailing list. We're here to help! This is simply a reminder of what we are\r\ngoing to look for before merging your code._\r\n\r\n- [X] I have read the `CONTRIBUTING.md` document\r\n- [X] I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)\r\n- [ ] All tests pass locally with my changes\r\n- [ ] I have added tests that prove my fix is effective or that my feature works\r\n- [ ] I have added necessary documentation (if appropriate)\r\n- [ ] Any dependent changes have been merged and published in related repositories\r\n\r\n## Further Comments\r\n\r\n`Producer.cs`\r\n```\r\n    public static class EntryPoint\r\n    {\r\n        public static void Main(string[] args)\r\n        {\r\n            var connectionFactory = new ConnectionFactory();\r\n            using (var connection = connectionFactory.CreateConnection())\r\n            {\r\n                using (var model = connection.CreateModel())\r\n                {\r\n                    model.ExchangeDeclare(\"Exchange\", \"direct\", true);\r\n                    for (var i = 0; i < 10000; ++i)\r\n                    {\r\n                        model.BasicPublish(\"Exchange\", \"#\", body: new byte[100000]);\r\n                    }\r\n                }\r\n            }   \r\n\r\n            Console.ReadKey();\r\n        }\r\n    }\r\n```\r\n\r\nBefore:\r\n![image](https://user-images.githubusercontent.com/664889/45472647-48b5b480-b74e-11e8-8733-94b83150aac2.png)\r\n[Before.zip](https://github.com/rabbitmq/rabbitmq-dotnet-client/files/2378097/Before.zip)\r\n\r\nAfter:\r\n![image](https://user-images.githubusercontent.com/664889/45471209-c7f4b980-b749-11e8-95b4-42b254b9e7ad.png)\r\n[After.zip](https://github.com/rabbitmq/rabbitmq-dotnet-client/files/2378078/After.zip)\r\n\r\nAs you can see, there are no more huge allocations of byte arrays except `Producer`. We can expect the same behaviour when size of body is less then 1 megabytes, because max size of pooled array is 2^20.\r\n\r\nOne more non-proven observation: after this fix `Producer.cs`(I recompiled it in Release mode) works significantly faster(before - 3.7s , after - 2.5s).\r\n\r\nPS It's an experiment. Please, do not merge it, because `PooledMemoryStream` is not properly tested. Also I'm sure we need to measure the performance too.\r\n"},"comment":{"url":"https://api.github.com/repos/rabbitmq/rabbitmq-dotnet-client/issues/comments/422182678","html_url":"https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/452#issuecomment-422182678","issue_url":"https://api.github.com/repos/rabbitmq/rabbitmq-dotnet-client/issues/452","id":422182678,"node_id":"MDEyOklzc3VlQ29tbWVudDQyMjE4MjY3OA==","user":{"login":"bording","id":753669,"node_id":"MDQ6VXNlcjc1MzY2OQ==","avatar_url":"https://avatars0.githubusercontent.com/u/753669?v=4","gravatar_id":"","url":"https://api.github.com/users/bording","html_url":"https://github.com/bording","followers_url":"https://api.github.com/users/bording/followers","following_url":"https://api.github.com/users/bording/following{/other_user}","gists_url":"https://api.github.com/users/bording/gists{/gist_id}","starred_url":"https://api.github.com/users/bording/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bording/subscriptions","organizations_url":"https://api.github.com/users/bording/orgs","repos_url":"https://api.github.com/users/bording/repos","events_url":"https://api.github.com/users/bording/events{/privacy}","received_events_url":"https://api.github.com/users/bording/received_events","type":"User","site_admin":false},"created_at":"2018-09-17T21:46:37Z","updated_at":"2018-09-17T21:46:37Z","author_association":"CONTRIBUTOR","body":"One reason that a reference to System.Buffers might not be a good idea is that it only ships a `netstandard1.1` and `netstandard2.0` version of the lib, so if you're using `net461` through `net471` you can end up in some weird pain because of how the .NET Standard 2.0 support for those versions works. Lots of difficult to solve binding redirect errors, etc."}}
{"actor":{"display_login":"bording"}}