{"action":"created","comment":{"url":"https://api.github.com/repos/pika/pika/pulls/comments/28408633","id":28408633,"diff_hunk":"@@ -239,7 +241,10 @@ def _get_error_code(self, error_value):\n         return None\n \n     def _flush_outbound(self):\n-        \"\"\"Call the state manager who will figure out that we need to write.\"\"\"\n+        \"\"\"write early, if the socket will take the data why not get it out\n+        there asap.\n+        \"\"\"\n+        self._handle_write()","path":"pika/adapters/base_connection.py","position":34,"original_position":26,"commit_id":"635957f8a780814cb6eaa91c886a891b31518662","original_commit_id":"576c1f09f5758a56965375167ae6944376ae735f","user":{"login":"wjps","id":11544560,"avatar_url":"https://avatars.githubusercontent.com/u/11544560?v=3","gravatar_id":"","url":"https://api.github.com/users/wjps","html_url":"https://github.com/wjps","followers_url":"https://api.github.com/users/wjps/followers","following_url":"https://api.github.com/users/wjps/following{/other_user}","gists_url":"https://api.github.com/users/wjps/gists{/gist_id}","starred_url":"https://api.github.com/users/wjps/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wjps/subscriptions","organizations_url":"https://api.github.com/users/wjps/orgs","repos_url":"https://api.github.com/users/wjps/repos","events_url":"https://api.github.com/users/wjps/events{/privacy}","received_events_url":"https://api.github.com/users/wjps/received_events","type":"User","site_admin":false},"body":"Yup sure, I'm not casting any aspersions in pika's `_handle_read` overhead.\r\n\r\nPerhaps my original comment was too flippant. I am actually _extremely_ interested in the performance of pika, but more than that, I'm interested in getting behaviour that is reliable, consistent and what a user might reasonably expect. I think this PR gets us a step closer to that goal, AND it is actually significantly more efficient than the existing code.\r\n\r\n\r\nLet's keep in mind that we're only talking about a situation here where the publisher is consistently generating messages faster than the network can send them, in this situation we would actually be pushing back using the backpressure detection, if it worked (I have a commit for that too).  In such a situation, taken to extreme, we're talking about swapping and potentially memory exhaustion, IMO a small additional overhead (1.4us* on my laptop) is probably not the first thing to worry about.\r\n\r\nFurthermore, scenarios such as the following continue to break down...\r\n\r\nImagine you have an application that is reading a large amount of data from a non rabbit source (say a large XML document retrieved over http, data from some sort of DB or even generated internally), as it processes that data it publishes a large number of messages. In this situation the code might well not drop into the ioloop until it's completed processing of all the data. On pika/master it would never attempt to send a single message until processing of the full document was complete whereas this PR would keep putting messages in the socket buffer whenever space was available. With your suggestion we would fill the socket buffer once and then never send another message until we finished processing the document.\r\n\r\nMy aim with this PR is to get the data on the wire ASAP to minimise latency and maximise opportunities for parallel-isation.\r\n\r\n*1.4us is the overhead on my laptop of attempting a 1byte write on a blocked socket in python, including catching and handling the generated exception.\r\n0.5us is the overhead of an equivalent write on an unblocked socket.\r\n4.3us is the overhead of dropping into the tornado ioloop and getting triggered when the socket becomes writeable as the existing code does, individually for EVERY frame sent.\r\n\r\n\r\n","created_at":"2015-04-15T10:26:09Z","updated_at":"2015-04-15T10:26:09Z","html_url":"https://github.com/pika/pika/pull/545#discussion_r28408633","pull_request_url":"https://api.github.com/repos/pika/pika/pulls/545","_links":{"self":{"href":"https://api.github.com/repos/pika/pika/pulls/comments/28408633"},"html":{"href":"https://github.com/pika/pika/pull/545#discussion_r28408633"},"pull_request":{"href":"https://api.github.com/repos/pika/pika/pulls/545"}}},"pull_request":{"url":"https://api.github.com/repos/pika/pika/pulls/545","id":33117340,"html_url":"https://github.com/pika/pika/pull/545","diff_url":"https://github.com/pika/pika/pull/545.diff","patch_url":"https://github.com/pika/pika/pull/545.patch","issue_url":"https://api.github.com/repos/pika/pika/issues/545","number":545,"state":"open","locked":false,"title":"write starvation fixes","user":{"login":"wjps","id":11544560,"avatar_url":"https://avatars.githubusercontent.com/u/11544560?v=3","gravatar_id":"","url":"https://api.github.com/users/wjps","html_url":"https://github.com/wjps","followers_url":"https://api.github.com/users/wjps/followers","following_url":"https://api.github.com/users/wjps/following{/other_user}","gists_url":"https://api.github.com/users/wjps/gists{/gist_id}","starred_url":"https://api.github.com/users/wjps/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wjps/subscriptions","organizations_url":"https://api.github.com/users/wjps/orgs","repos_url":"https://api.github.com/users/wjps/repos","events_url":"https://api.github.com/users/wjps/events{/privacy}","received_events_url":"https://api.github.com/users/wjps/received_events","type":"User","site_admin":false},"body":"Hi, this contains some changes to the way writes are sent in pika that addresses some issues we were seeing when using the tornado adapter. There's more detail in the individual commit messages but the basic premise is that we should get the data out on the wire as quickly as possible. Unfortunately the use sendall() and timeouts isn't really suited to that way of working so the socket has been changed to non-blocking (after the connect sequence) and we simply handle the write errors when the socket buffer is full.\r\n\r\nThe changes seem to be generally applicable to other connection adapters and in fact there seems to be a bug with BlockingConnections on master that is fixed by applying these changes.\r\n\r\nI've load tested the various adapters and In general I see much more consistent and reliable behaviour now (no random timeouts, pipeline stalls etc).\r\n\r\nCheers,\r\n\r\nWill","created_at":"2015-04-12T09:20:37Z","updated_at":"2015-04-15T10:26:09Z","closed_at":null,"merged_at":null,"merge_commit_sha":"86ac1808a5964837c591cd3300ea9e61569ff8f2","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/pika/pika/pulls/545/commits","review_comments_url":"https://api.github.com/repos/pika/pika/pulls/545/comments","review_comment_url":"https://api.github.com/repos/pika/pika/pulls/comments{/number}","comments_url":"https://api.github.com/repos/pika/pika/issues/545/comments","statuses_url":"https://api.github.com/repos/pika/pika/statuses/635957f8a780814cb6eaa91c886a891b31518662","head":{"label":"wjps:master","ref":"master","sha":"635957f8a780814cb6eaa91c886a891b31518662","user":{"login":"wjps","id":11544560,"avatar_url":"https://avatars.githubusercontent.com/u/11544560?v=3","gravatar_id":"","url":"https://api.github.com/users/wjps","html_url":"https://github.com/wjps","followers_url":"https://api.github.com/users/wjps/followers","following_url":"https://api.github.com/users/wjps/following{/other_user}","gists_url":"https://api.github.com/users/wjps/gists{/gist_id}","starred_url":"https://api.github.com/users/wjps/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wjps/subscriptions","organizations_url":"https://api.github.com/users/wjps/orgs","repos_url":"https://api.github.com/users/wjps/repos","events_url":"https://api.github.com/users/wjps/events{/privacy}","received_events_url":"https://api.github.com/users/wjps/received_events","type":"User","site_admin":false},"repo":{"id":32480142,"name":"pika","full_name":"wjps/pika","owner":{"login":"wjps","id":11544560,"avatar_url":"https://avatars.githubusercontent.com/u/11544560?v=3","gravatar_id":"","url":"https://api.github.com/users/wjps","html_url":"https://github.com/wjps","followers_url":"https://api.github.com/users/wjps/followers","following_url":"https://api.github.com/users/wjps/following{/other_user}","gists_url":"https://api.github.com/users/wjps/gists{/gist_id}","starred_url":"https://api.github.com/users/wjps/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wjps/subscriptions","organizations_url":"https://api.github.com/users/wjps/orgs","repos_url":"https://api.github.com/users/wjps/repos","events_url":"https://api.github.com/users/wjps/events{/privacy}","received_events_url":"https://api.github.com/users/wjps/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/wjps/pika","description":"Pure Python RabbitMQ/AMQP 0-9-1 client library","fork":true,"url":"https://api.github.com/repos/wjps/pika","forks_url":"https://api.github.com/repos/wjps/pika/forks","keys_url":"https://api.github.com/repos/wjps/pika/keys{/key_id}","collaborators_url":"https://api.github.com/repos/wjps/pika/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/wjps/pika/teams","hooks_url":"https://api.github.com/repos/wjps/pika/hooks","issue_events_url":"https://api.github.com/repos/wjps/pika/issues/events{/number}","events_url":"https://api.github.com/repos/wjps/pika/events","assignees_url":"https://api.github.com/repos/wjps/pika/assignees{/user}","branches_url":"https://api.github.com/repos/wjps/pika/branches{/branch}","tags_url":"https://api.github.com/repos/wjps/pika/tags","blobs_url":"https://api.github.com/repos/wjps/pika/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/wjps/pika/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/wjps/pika/git/refs{/sha}","trees_url":"https://api.github.com/repos/wjps/pika/git/trees{/sha}","statuses_url":"https://api.github.com/repos/wjps/pika/statuses/{sha}","languages_url":"https://api.github.com/repos/wjps/pika/languages","stargazers_url":"https://api.github.com/repos/wjps/pika/stargazers","contributors_url":"https://api.github.com/repos/wjps/pika/contributors","subscribers_url":"https://api.github.com/repos/wjps/pika/subscribers","subscription_url":"https://api.github.com/repos/wjps/pika/subscription","commits_url":"https://api.github.com/repos/wjps/pika/commits{/sha}","git_commits_url":"https://api.github.com/repos/wjps/pika/git/commits{/sha}","comments_url":"https://api.github.com/repos/wjps/pika/comments{/number}","issue_comment_url":"https://api.github.com/repos/wjps/pika/issues/comments{/number}","contents_url":"https://api.github.com/repos/wjps/pika/contents/{+path}","compare_url":"https://api.github.com/repos/wjps/pika/compare/{base}...{head}","merges_url":"https://api.github.com/repos/wjps/pika/merges","archive_url":"https://api.github.com/repos/wjps/pika/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/wjps/pika/downloads","issues_url":"https://api.github.com/repos/wjps/pika/issues{/number}","pulls_url":"https://api.github.com/repos/wjps/pika/pulls{/number}","milestones_url":"https://api.github.com/repos/wjps/pika/milestones{/number}","notifications_url":"https://api.github.com/repos/wjps/pika/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/wjps/pika/labels{/name}","releases_url":"https://api.github.com/repos/wjps/pika/releases{/id}","created_at":"2015-03-18T19:38:36Z","updated_at":"2015-04-14T10:34:23Z","pushed_at":"2015-04-14T10:34:23Z","git_url":"git://github.com/wjps/pika.git","ssh_url":"git@github.com:wjps/pika.git","clone_url":"https://github.com/wjps/pika.git","svn_url":"https://github.com/wjps/pika","homepage":"http://pika.readthedocs.org","size":5417,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"pika:master","ref":"master","sha":"409670bdbea13d11a43d3d02162d49a200e450a5","user":{"login":"pika","id":660428,"avatar_url":"https://avatars.githubusercontent.com/u/660428?v=3","gravatar_id":"","url":"https://api.github.com/users/pika","html_url":"https://github.com/pika","followers_url":"https://api.github.com/users/pika/followers","following_url":"https://api.github.com/users/pika/following{/other_user}","gists_url":"https://api.github.com/users/pika/gists{/gist_id}","starred_url":"https://api.github.com/users/pika/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pika/subscriptions","organizations_url":"https://api.github.com/users/pika/orgs","repos_url":"https://api.github.com/users/pika/repos","events_url":"https://api.github.com/users/pika/events{/privacy}","received_events_url":"https://api.github.com/users/pika/received_events","type":"Organization","site_admin":false},"repo":{"id":342869,"name":"pika","full_name":"pika/pika","owner":{"login":"pika","id":660428,"avatar_url":"https://avatars.githubusercontent.com/u/660428?v=3","gravatar_id":"","url":"https://api.github.com/users/pika","html_url":"https://github.com/pika","followers_url":"https://api.github.com/users/pika/followers","following_url":"https://api.github.com/users/pika/following{/other_user}","gists_url":"https://api.github.com/users/pika/gists{/gist_id}","starred_url":"https://api.github.com/users/pika/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pika/subscriptions","organizations_url":"https://api.github.com/users/pika/orgs","repos_url":"https://api.github.com/users/pika/repos","events_url":"https://api.github.com/users/pika/events{/privacy}","received_events_url":"https://api.github.com/users/pika/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/pika/pika","description":"Pure Python RabbitMQ/AMQP 0-9-1 client library","fork":false,"url":"https://api.github.com/repos/pika/pika","forks_url":"https://api.github.com/repos/pika/pika/forks","keys_url":"https://api.github.com/repos/pika/pika/keys{/key_id}","collaborators_url":"https://api.github.com/repos/pika/pika/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/pika/pika/teams","hooks_url":"https://api.github.com/repos/pika/pika/hooks","issue_events_url":"https://api.github.com/repos/pika/pika/issues/events{/number}","events_url":"https://api.github.com/repos/pika/pika/events","assignees_url":"https://api.github.com/repos/pika/pika/assignees{/user}","branches_url":"https://api.github.com/repos/pika/pika/branches{/branch}","tags_url":"https://api.github.com/repos/pika/pika/tags","blobs_url":"https://api.github.com/repos/pika/pika/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/pika/pika/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/pika/pika/git/refs{/sha}","trees_url":"https://api.github.com/repos/pika/pika/git/trees{/sha}","statuses_url":"https://api.github.com/repos/pika/pika/statuses/{sha}","languages_url":"https://api.github.com/repos/pika/pika/languages","stargazers_url":"https://api.github.com/repos/pika/pika/stargazers","contributors_url":"https://api.github.com/repos/pika/pika/contributors","subscribers_url":"https://api.github.com/repos/pika/pika/subscribers","subscription_url":"https://api.github.com/repos/pika/pika/subscription","commits_url":"https://api.github.com/repos/pika/pika/commits{/sha}","git_commits_url":"https://api.github.com/repos/pika/pika/git/commits{/sha}","comments_url":"https://api.github.com/repos/pika/pika/comments{/number}","issue_comment_url":"https://api.github.com/repos/pika/pika/issues/comments{/number}","contents_url":"https://api.github.com/repos/pika/pika/contents/{+path}","compare_url":"https://api.github.com/repos/pika/pika/compare/{base}...{head}","merges_url":"https://api.github.com/repos/pika/pika/merges","archive_url":"https://api.github.com/repos/pika/pika/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/pika/pika/downloads","issues_url":"https://api.github.com/repos/pika/pika/issues{/number}","pulls_url":"https://api.github.com/repos/pika/pika/pulls{/number}","milestones_url":"https://api.github.com/repos/pika/pika/milestones{/number}","notifications_url":"https://api.github.com/repos/pika/pika/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/pika/pika/labels{/name}","releases_url":"https://api.github.com/repos/pika/pika/releases{/id}","created_at":"2009-10-19T23:22:02Z","updated_at":"2015-04-14T01:27:42Z","pushed_at":"2015-04-14T10:34:23Z","git_url":"git://github.com/pika/pika.git","ssh_url":"git@github.com:pika/pika.git","clone_url":"https://github.com/pika/pika.git","svn_url":"https://github.com/pika/pika","homepage":"http://pika.readthedocs.org","size":19379,"stargazers_count":780,"watchers_count":780,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":282,"mirror_url":null,"open_issues_count":87,"forks":282,"open_issues":87,"watchers":780,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/pika/pika/pulls/545"},"html":{"href":"https://github.com/pika/pika/pull/545"},"issue":{"href":"https://api.github.com/repos/pika/pika/issues/545"},"comments":{"href":"https://api.github.com/repos/pika/pika/issues/545/comments"},"review_comments":{"href":"https://api.github.com/repos/pika/pika/pulls/545/comments"},"review_comment":{"href":"https://api.github.com/repos/pika/pika/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/pika/pika/pulls/545/commits"},"statuses":{"href":"https://api.github.com/repos/pika/pika/statuses/635957f8a780814cb6eaa91c886a891b31518662"}}}}