{"action":"created","issue":{"url":"https://api.github.com/repos/grpc/grpc-java/issues/2240","repository_url":"https://api.github.com/repos/grpc/grpc-java","labels_url":"https://api.github.com/repos/grpc/grpc-java/issues/2240/labels{/name}","comments_url":"https://api.github.com/repos/grpc/grpc-java/issues/2240/comments","events_url":"https://api.github.com/repos/grpc/grpc-java/issues/2240/events","html_url":"https://github.com/grpc/grpc-java/pull/2240","id":175674130,"number":2240,"title":"netty: Recycle command objects on the WriteQueue","user":{"login":"carl-mastrangelo","id":8943572,"avatar_url":"https://avatars.githubusercontent.com/u/8943572?v=3","gravatar_id":"","url":"https://api.github.com/users/carl-mastrangelo","html_url":"https://github.com/carl-mastrangelo","followers_url":"https://api.github.com/users/carl-mastrangelo/followers","following_url":"https://api.github.com/users/carl-mastrangelo/following{/other_user}","gists_url":"https://api.github.com/users/carl-mastrangelo/gists{/gist_id}","starred_url":"https://api.github.com/users/carl-mastrangelo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/carl-mastrangelo/subscriptions","organizations_url":"https://api.github.com/users/carl-mastrangelo/orgs","repos_url":"https://api.github.com/users/carl-mastrangelo/repos","events_url":"https://api.github.com/users/carl-mastrangelo/events{/privacy}","received_events_url":"https://api.github.com/users/carl-mastrangelo/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/grpc/grpc-java/labels/performance","name":"performance","color":"5319e7"}],"state":"open","locked":false,"assignee":{"login":"buchgr","id":1388179,"avatar_url":"https://avatars.githubusercontent.com/u/1388179?v=3","gravatar_id":"","url":"https://api.github.com/users/buchgr","html_url":"https://github.com/buchgr","followers_url":"https://api.github.com/users/buchgr/followers","following_url":"https://api.github.com/users/buchgr/following{/other_user}","gists_url":"https://api.github.com/users/buchgr/gists{/gist_id}","starred_url":"https://api.github.com/users/buchgr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/buchgr/subscriptions","organizations_url":"https://api.github.com/users/buchgr/orgs","repos_url":"https://api.github.com/users/buchgr/repos","events_url":"https://api.github.com/users/buchgr/events{/privacy}","received_events_url":"https://api.github.com/users/buchgr/received_events","type":"User","site_admin":false},"assignees":[{"login":"buchgr","id":1388179,"avatar_url":"https://avatars.githubusercontent.com/u/1388179?v=3","gravatar_id":"","url":"https://api.github.com/users/buchgr","html_url":"https://github.com/buchgr","followers_url":"https://api.github.com/users/buchgr/followers","following_url":"https://api.github.com/users/buchgr/following{/other_user}","gists_url":"https://api.github.com/users/buchgr/gists{/gist_id}","starred_url":"https://api.github.com/users/buchgr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/buchgr/subscriptions","organizations_url":"https://api.github.com/users/buchgr/orgs","repos_url":"https://api.github.com/users/buchgr/repos","events_url":"https://api.github.com/users/buchgr/events{/privacy}","received_events_url":"https://api.github.com/users/buchgr/received_events","type":"User","site_admin":false}],"milestone":null,"comments":3,"created_at":"2016-09-08T06:20:45Z","updated_at":"2016-09-13T01:06:11Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/grpc/grpc-java/pulls/2240","html_url":"https://github.com/grpc/grpc-java/pull/2240","diff_url":"https://github.com/grpc/grpc-java/pull/2240.diff","patch_url":"https://github.com/grpc/grpc-java/pull/2240.patch"},"body":"WriteQueue may not be long for this world, but we have to live with it while it is here.\r\n\r\nThis change makes it so that QueueCommand objects are recycled from a pool of \"UnionCommand\" objects, which represent all possible commands.  They all share a single small set of variables.  Each command is actually a tagging interface with the appropriate getters.  \r\n\r\nBecause each of these objects was destined to be sent across a thread, there is basically no chance they could be stack or TLAB allocated.  I suspect the objects will flow from the application thread to the network thread, and hope that Recycler steals chunks of them back over as needed.  \r\n\r\nI don't expect this to have a dramatic impact, but it should reduce some of the noise in the allocation profiler.  By my count it saves about 2% of object allocation.\r\n\r\nSome nice things about this:\r\n\r\n- The if-else chain in the Handlers is gone!  It is replaced by an enum switch\r\n- Because Union command implements all command interfaces, it can be passed to other functions without casting\r\n- The other functions always ask for the subtype, so they don't call the wrong method.\r\n- All commands now implement equals automatically. \r\n- SendGrpcFrameCommand was previous in a different part of the hierarchy because it extended ByteBufHolder, and couldn't extend AbstractCommand.  The hierarchy is now uniform\r\n\r\ncc: @louiscryan "},"comment":{"url":"https://api.github.com/repos/grpc/grpc-java/issues/comments/246543043","html_url":"https://github.com/grpc/grpc-java/pull/2240#issuecomment-246543043","issue_url":"https://api.github.com/repos/grpc/grpc-java/issues/2240","id":246543043,"user":{"login":"carl-mastrangelo","id":8943572,"avatar_url":"https://avatars.githubusercontent.com/u/8943572?v=3","gravatar_id":"","url":"https://api.github.com/users/carl-mastrangelo","html_url":"https://github.com/carl-mastrangelo","followers_url":"https://api.github.com/users/carl-mastrangelo/followers","following_url":"https://api.github.com/users/carl-mastrangelo/following{/other_user}","gists_url":"https://api.github.com/users/carl-mastrangelo/gists{/gist_id}","starred_url":"https://api.github.com/users/carl-mastrangelo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/carl-mastrangelo/subscriptions","organizations_url":"https://api.github.com/users/carl-mastrangelo/orgs","repos_url":"https://api.github.com/users/carl-mastrangelo/repos","events_url":"https://api.github.com/users/carl-mastrangelo/events{/privacy}","received_events_url":"https://api.github.com/users/carl-mastrangelo/received_events","type":"User","site_admin":false},"created_at":"2016-09-13T01:06:11Z","updated_at":"2016-09-13T01:06:11Z","body":"Okay, some things.  I tried hacking this change to use  `new UnionCommand(null)` instead of `recycler.get()`.  Also I made `recycle` do nothing, which effectively is what happens before this PR.\r\n\r\nAllocation time with new:\r\n50%ile Latency (in nanos):     72\r\n90%ile Latency (in nanos):     121\r\n95%ile Latency (in nanos):     135\r\n99%ile Latency (in nanos):     198\r\n99.9%ile Latency (in nanos):   11031\r\nMaximum Latency (in nanos):    51391\r\n\r\nAllocation with Recycler:\r\n50%ile Latency (in nanos):     168\r\n90%ile Latency (in nanos):     449\r\n95%ile Latency (in nanos):     499\r\n99%ile Latency (in nanos):     846\r\n99.9%ile Latency (in nanos):   48127\r\nMaximum Latency (in nanos):    60543\r\n\r\n`handle.recycle(this)`:\r\n50%ile Latency (in nanos):     90\r\n90%ile Latency (in nanos):     159\r\n95%ile Latency (in nanos):     173\r\n99%ile Latency (in nanos):     313\r\n99.9%ile Latency (in nanos):   14663\r\nMaximum Latency (in nanos):    23263\r\n\r\n\r\n\r\nI'm getting suspicious that recycler may not be a good fit for such tiny objects\r\nI didn't measure the GC, but those higher latencies don't look good.  \r\n\r\n\r\n\r\n"}}
{"actor":{"display_login":"carl-mastrangelo"}}