{"action":"created","issue":{"url":"https://api.github.com/repos/grpc/grpc-go/issues/1482","repository_url":"https://api.github.com/repos/grpc/grpc-go","labels_url":"https://api.github.com/repos/grpc/grpc-go/issues/1482/labels{/name}","comments_url":"https://api.github.com/repos/grpc/grpc-go/issues/1482/comments","events_url":"https://api.github.com/repos/grpc/grpc-go/issues/1482/events","html_url":"https://github.com/grpc/grpc-go/pull/1482","id":253151809,"number":1482,"title":"grpc: zero-alloc {Client,Server}Transport.Write paths","user":{"login":"irfansharif","id":10536690,"avatar_url":"https://avatars3.githubusercontent.com/u/10536690?v=4","gravatar_id":"","url":"https://api.github.com/users/irfansharif","html_url":"https://github.com/irfansharif","followers_url":"https://api.github.com/users/irfansharif/followers","following_url":"https://api.github.com/users/irfansharif/following{/other_user}","gists_url":"https://api.github.com/users/irfansharif/gists{/gist_id}","starred_url":"https://api.github.com/users/irfansharif/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/irfansharif/subscriptions","organizations_url":"https://api.github.com/users/irfansharif/orgs","repos_url":"https://api.github.com/users/irfansharif/repos","events_url":"https://api.github.com/users/irfansharif/events{/privacy}","received_events_url":"https://api.github.com/users/irfansharif/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":{"login":"MakMukhi","id":20670279,"avatar_url":"https://avatars1.githubusercontent.com/u/20670279?v=4","gravatar_id":"","url":"https://api.github.com/users/MakMukhi","html_url":"https://github.com/MakMukhi","followers_url":"https://api.github.com/users/MakMukhi/followers","following_url":"https://api.github.com/users/MakMukhi/following{/other_user}","gists_url":"https://api.github.com/users/MakMukhi/gists{/gist_id}","starred_url":"https://api.github.com/users/MakMukhi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MakMukhi/subscriptions","organizations_url":"https://api.github.com/users/MakMukhi/orgs","repos_url":"https://api.github.com/users/MakMukhi/repos","events_url":"https://api.github.com/users/MakMukhi/events{/privacy}","received_events_url":"https://api.github.com/users/MakMukhi/received_events","type":"User","site_admin":false},"assignees":[{"login":"MakMukhi","id":20670279,"avatar_url":"https://avatars1.githubusercontent.com/u/20670279?v=4","gravatar_id":"","url":"https://api.github.com/users/MakMukhi","html_url":"https://github.com/MakMukhi","followers_url":"https://api.github.com/users/MakMukhi/followers","following_url":"https://api.github.com/users/MakMukhi/following{/other_user}","gists_url":"https://api.github.com/users/MakMukhi/gists{/gist_id}","starred_url":"https://api.github.com/users/MakMukhi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MakMukhi/subscriptions","organizations_url":"https://api.github.com/users/MakMukhi/orgs","repos_url":"https://api.github.com/users/MakMukhi/repos","events_url":"https://api.github.com/users/MakMukhi/events{/privacy}","received_events_url":"https://api.github.com/users/MakMukhi/received_events","type":"User","site_admin":false}],"milestone":null,"comments":2,"created_at":"2017-08-27T09:17:37Z","updated_at":"2017-08-28T23:11:27Z","closed_at":"2017-08-28T23:11:27Z","pull_request":{"url":"https://api.github.com/repos/grpc/grpc-go/pulls/1482","html_url":"https://github.com/grpc/grpc-go/pull/1482","diff_url":"https://github.com/grpc/grpc-go/pull/1482.diff","patch_url":"https://github.com/grpc/grpc-go/pull/1482.patch"},"body":"#1427 avoided the allocation of an extra buffer by re-using the buffer\r\ngenerated by the Compressor. It did so by separating out the header and data\r\nbuffers and delegating to the transport layer the chunking of the two into\r\nappropriately sized frames.\r\n\r\nAt the transport layer however, given the affinity to generate 16KB frames, we\r\nre-concatenated the header and data buffers and subsequently flushed out\r\ncontiguous chunks. Given that the header buffer was always 5B in size, merging\r\nthe data bytes into the header buffer necessarily involved allocations and\r\ncopying. In practice for every {Client,Server}Transport.Write we would allocate\r\nand copy from 0 to ~16KB bytes of the data buffer into the header buffer. \r\n   \r\nBy reverting #1427 and re-working the implementation of rpc_util.go:encode, we\r\ncan stitch a single buffer comprised of both the serialized{,compressed} message\r\nwith the prepended message header. The transport layer is again agnostic to\r\nthe exact delineation and can simply write out 16KB chunks as needed (from the \r\nsingle buffer) without having to copy/allocate.\r\nAll throughout we are still re-using the buffer generated during Compression, \r\neffectively what  #1427 gave us, while retaining the simpler Transport API.\r\n\r\n---\r\n\r\nWe can now use a sync.Pool free list for oft-allocated byte.Buffers. This\r\nmarkedly reduces allocations for when compression is enabled.\r\n\r\n---\r\n\r\nReverts \"Remove buf copy when the compressor exist (#1427)\",  commit 01089b297206d04bac5d3c3805a51904fa44056c.\r\n"},"comment":{"url":"https://api.github.com/repos/grpc/grpc-go/issues/comments/325508487","html_url":"https://github.com/grpc/grpc-go/pull/1482#issuecomment-325508487","issue_url":"https://api.github.com/repos/grpc/grpc-go/issues/1482","id":325508487,"user":{"login":"dfawley","id":26072277,"avatar_url":"https://avatars1.githubusercontent.com/u/26072277?v=4","gravatar_id":"","url":"https://api.github.com/users/dfawley","html_url":"https://github.com/dfawley","followers_url":"https://api.github.com/users/dfawley/followers","following_url":"https://api.github.com/users/dfawley/following{/other_user}","gists_url":"https://api.github.com/users/dfawley/gists{/gist_id}","starred_url":"https://api.github.com/users/dfawley/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dfawley/subscriptions","organizations_url":"https://api.github.com/users/dfawley/orgs","repos_url":"https://api.github.com/users/dfawley/repos","events_url":"https://api.github.com/users/dfawley/events{/privacy}","received_events_url":"https://api.github.com/users/dfawley/received_events","type":"User","site_admin":false},"created_at":"2017-08-28T23:11:27Z","updated_at":"2017-08-28T23:11:27Z","body":"The issue with this change is that it drastically increases allocations _and_ copying for large messages without compression enabled (the justification for the optimization it rolls back).  We should be able to do better for both use cases.  @MakMukhi is working on such improvements, but I don't believe we can take this PR as-is in the interim, given the significant cost it adds to non-compression workloads.\r\n\r\nIf you need a short term improvement, maybe return an empty header slice from encode(), and include the header info in the data buffer when compression is enabled?"}}
{"actor":{"display_login":"dfawley"}}