GitHub PullRequestEvent at 2014-05-30 15:05:39
Repository: neovim
Actor: aktau
Payload: {"action":"opened","number":783,"pull_request":{"url":"https://api.github.com/repos/neovim/neovim/pulls/783","id":16537484,"html_url":"https://github.com/neovim/neovim/pull/783","diff_url":"https://github.com/neovim/neovim/pull/783.diff","patch_url":"https://github.com/neovim/neovim/pull/783.patch","issue_url":"https://api.github.com/repos/neovim/neovim/issues/783","number":783,"state":"open","title":"[RFC] Nul terminate pascal strings","user":{"login":"aktau","id":189413,"avatar_url":"https://avatars.githubusercontent.com/u/189413?","gravatar_id":"7b04c723316dd727866faa8d79962e77","url":"https://api.github.com/users/aktau","html_url":"https://github.com/aktau","followers_url":"https://api.github.com/users/aktau/followers","following_url":"https://api.github.com/users/aktau/following{/other_user}","gists_url":"https://api.github.com/users/aktau/gists{/gist_id}","starred_url":"https://api.github.com/users/aktau/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aktau/subscriptions","organizations_url":"https://api.github.com/users/aktau/orgs","repos_url":"https://api.github.com/users/aktau/repos","events_url":"https://api.github.com/users/aktau/events{/privacy}","received_events_url":"https://api.github.com/users/aktau/received_events","type":"User","site_admin":false},"body":"I would like to request comments on this one.\r\n\r\nLowering the amounts malloc/free pairs is a good way to be nicer to the system, and in this case it saves lines of code too. We used to have:\r\n\r\n1. libuv incoming buffer (I don't know if that's reused over different incoming messages, I hope so)\r\n2. msgpack decoding buffer (don't know if reused either)\r\n3. msgpack element to String (`xmemdup`)\r\n4. String to cstr (`xstrndup`), which was doubly ineffecient because it also scans for `\\0`, which it won't find (in the general case).\r\n\r\nThe last step is what I aim to eliminate. In theory, the 3rd step is the only one where I see problems for eliminating continuous allocation. But if it comes to that, we can create a small pool of strings to reuse (and when there's nothing in the pool on an incoming message, either enlarge the pool or fallback to `xmemdup`/`free`).\r\n\r\nPing @tarruda since this concerns the API, and @justinmk / @philix because I believe you guys have been dabbling in the same area of the code lately as well.","created_at":"2014-05-30T15:05:38Z","updated_at":"2014-05-30T15:05:38Z","closed_at":null,"merged_at":null,"merge_commit_sha":null,"assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/neovim/neovim/pulls/783/commits","review_comments_url":"https://api.github.com/repos/neovim/neovim/pulls/783/comments","review_comment_url":"https://api.github.com/repos/neovim/neovim/pulls/comments/{number}","comments_url":"https://api.github.com/repos/neovim/neovim/issues/783/comments","statuses_url":"https://api.github.com/repos/neovim/neovim/statuses/b41af754c732da0aea520b725930c7457e9eb5ab","head":{"label":"aktau:nul-terminate-pascal-strings","ref":"nul-terminate-pascal-strings","sha":"b41af754c732da0aea520b725930c7457e9eb5ab","user":{"login":"aktau","id":189413,"avatar_url":"https://avatars.githubusercontent.com/u/189413?","gravatar_id":"7b04c723316dd727866faa8d79962e77","url":"https://api.github.com/users/aktau","html_url":"https://github.com/aktau","followers_url":"https://api.github.com/users/aktau/followers","following_url":"https://api.github.com/users/aktau/following{/other_user}","gists_url":"https://api.github.com/users/aktau/gists{/gist_id}","starred_url":"https://api.github.com/users/aktau/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aktau/subscriptions","organizations_url":"https://api.github.com/users/aktau/orgs","repos_url":"https://api.github.com/users/aktau/repos","events_url":"https://api.github.com/users/aktau/events{/privacy}","received_events_url":"https://api.github.com/users/aktau/received_events","type":"User","site_admin":false},"repo":{"id":17257036,"name":"neovim","full_name":"aktau/neovim","owner":{"login":"aktau","id":189413,"avatar_url":"https://avatars.githubusercontent.com/u/189413?","gravatar_id":"7b04c723316dd727866faa8d79962e77","url":"https://api.github.com/users/aktau","html_url":"https://github.com/aktau","followers_url":"https://api.github.com/users/aktau/followers","following_url":"https://api.github.com/users/aktau/following{/other_user}","gists_url":"https://api.github.com/users/aktau/gists{/gist_id}","starred_url":"https://api.github.com/users/aktau/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aktau/subscriptions","organizations_url":"https://api.github.com/users/aktau/orgs","repos_url":"https://api.github.com/users/aktau/repos","events_url":"https://api.github.com/users/aktau/events{/privacy}","received_events_url":"https://api.github.com/users/aktau/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/aktau/neovim","description":"vim's rebirth for the 21st century","fork":true,"url":"https://api.github.com/repos/aktau/neovim","forks_url":"https://api.github.com/repos/aktau/neovim/forks","keys_url":"https://api.github.com/repos/aktau/neovim/keys{/key_id}","collaborators_url":"https://api.github.com/repos/aktau/neovim/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/aktau/neovim/teams","hooks_url":"https://api.github.com/repos/aktau/neovim/hooks","issue_events_url":"https://api.github.com/repos/aktau/neovim/issues/events{/number}","events_url":"https://api.github.com/repos/aktau/neovim/events","assignees_url":"https://api.github.com/repos/aktau/neovim/assignees{/user}","branches_url":"https://api.github.com/repos/aktau/neovim/branches{/branch}","tags_url":"https://api.github.com/repos/aktau/neovim/tags","blobs_url":"https://api.github.com/repos/aktau/neovim/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/aktau/neovim/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/aktau/neovim/git/refs{/sha}","trees_url":"https://api.github.com/repos/aktau/neovim/git/trees{/sha}","statuses_url":"https://api.github.com/repos/aktau/neovim/statuses/{sha}","languages_url":"https://api.github.com/repos/aktau/neovim/languages","stargazers_url":"https://api.github.com/repos/aktau/neovim/stargazers","contributors_url":"https://api.github.com/repos/aktau/neovim/contributors","subscribers_url":"https://api.github.com/repos/aktau/neovim/subscribers","subscription_url":"https://api.github.com/repos/aktau/neovim/subscription","commits_url":"https://api.github.com/repos/aktau/neovim/commits{/sha}","git_commits_url":"https://api.github.com/repos/aktau/neovim/git/commits{/sha}","comments_url":"https://api.github.com/repos/aktau/neovim/comments{/number}","issue_comment_url":"https://api.github.com/repos/aktau/neovim/issues/comments/{number}","contents_url":"https://api.github.com/repos/aktau/neovim/contents/{+path}","compare_url":"https://api.github.com/repos/aktau/neovim/compare/{base}...{head}","merges_url":"https://api.github.com/repos/aktau/neovim/merges","archive_url":"https://api.github.com/repos/aktau/neovim/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/aktau/neovim/downloads","issues_url":"https://api.github.com/repos/aktau/neovim/issues{/number}","pulls_url":"https://api.github.com/repos/aktau/neovim/pulls{/number}","milestones_url":"https://api.github.com/repos/aktau/neovim/milestones{/number}","notifications_url":"https://api.github.com/repos/aktau/neovim/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/aktau/neovim/labels{/name}","releases_url":"https://api.github.com/repos/aktau/neovim/releases{/id}","created_at":"2014-02-27T17:13:14Z","updated_at":"2014-05-30T15:05:38Z","pushed_at":"2014-05-30T14:59:14Z","git_url":"git://github.com/aktau/neovim.git","ssh_url":"git@github.com:aktau/neovim.git","clone_url":"https://github.com/aktau/neovim.git","svn_url":"https://github.com/aktau/neovim","homepage":"http://neovim.org/","size":15145,"stargazers_count":0,"watchers_count":0,"language":"C","has_issues":false,"has_downloads":true,"has_wiki":true,"forks_count":1,"mirror_url":null,"open_issues_count":0,"forks":1,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"neovim:master","ref":"master","sha":"a581173e715af4f7ef4e6be232ed828bd7dea9b8","user":{"login":"neovim","id":6471485,"avatar_url":"https://avatars.githubusercontent.com/u/6471485?","gravatar_id":"294926ca6818f582b716e868d6c6f985","url":"https://api.github.com/users/neovim","html_url":"https://github.com/neovim","followers_url":"https://api.github.com/users/neovim/followers","following_url":"https://api.github.com/users/neovim/following{/other_user}","gists_url":"https://api.github.com/users/neovim/gists{/gist_id}","starred_url":"https://api.github.com/users/neovim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/neovim/subscriptions","organizations_url":"https://api.github.com/users/neovim/orgs","repos_url":"https://api.github.com/users/neovim/repos","events_url":"https://api.github.com/users/neovim/events{/privacy}","received_events_url":"https://api.github.com/users/neovim/received_events","type":"Organization","site_admin":false},"repo":{"id":16408992,"name":"neovim","full_name":"neovim/neovim","owner":{"login":"neovim","id":6471485,"avatar_url":"https://avatars.githubusercontent.com/u/6471485?","gravatar_id":"294926ca6818f582b716e868d6c6f985","url":"https://api.github.com/users/neovim","html_url":"https://github.com/neovim","followers_url":"https://api.github.com/users/neovim/followers","following_url":"https://api.github.com/users/neovim/following{/other_user}","gists_url":"https://api.github.com/users/neovim/gists{/gist_id}","starred_url":"https://api.github.com/users/neovim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/neovim/subscriptions","organizations_url":"https://api.github.com/users/neovim/orgs","repos_url":"https://api.github.com/users/neovim/repos","events_url":"https://api.github.com/users/neovim/events{/privacy}","received_events_url":"https://api.github.com/users/neovim/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/neovim/neovim","description":"vim's rebirth for the 21st century","fork":false,"url":"https://api.github.com/repos/neovim/neovim","forks_url":"https://api.github.com/repos/neovim/neovim/forks","keys_url":"https://api.github.com/repos/neovim/neovim/keys{/key_id}","collaborators_url":"https://api.github.com/repos/neovim/neovim/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/neovim/neovim/teams","hooks_url":"https://api.github.com/repos/neovim/neovim/hooks","issue_events_url":"https://api.github.com/repos/neovim/neovim/issues/events{/number}","events_url":"https://api.github.com/repos/neovim/neovim/events","assignees_url":"https://api.github.com/repos/neovim/neovim/assignees{/user}","branches_url":"https://api.github.com/repos/neovim/neovim/branches{/branch}","tags_url":"https://api.github.com/repos/neovim/neovim/tags","blobs_url":"https://api.github.com/repos/neovim/neovim/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/neovim/neovim/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/neovim/neovim/git/refs{/sha}","trees_url":"https://api.github.com/repos/neovim/neovim/git/trees{/sha}","statuses_url":"https://api.github.com/repos/neovim/neovim/statuses/{sha}","languages_url":"https://api.github.com/repos/neovim/neovim/languages","stargazers_url":"https://api.github.com/repos/neovim/neovim/stargazers","contributors_url":"https://api.github.com/repos/neovim/neovim/contributors","subscribers_url":"https://api.github.com/repos/neovim/neovim/subscribers","subscription_url":"https://api.github.com/repos/neovim/neovim/subscription","commits_url":"https://api.github.com/repos/neovim/neovim/commits{/sha}","git_commits_url":"https://api.github.com/repos/neovim/neovim/git/commits{/sha}","comments_url":"https://api.github.com/repos/neovim/neovim/comments{/number}","issue_comment_url":"https://api.github.com/repos/neovim/neovim/issues/comments/{number}","contents_url":"https://api.github.com/repos/neovim/neovim/contents/{+path}","compare_url":"https://api.github.com/repos/neovim/neovim/compare/{base}...{head}","merges_url":"https://api.github.com/repos/neovim/neovim/merges","archive_url":"https://api.github.com/repos/neovim/neovim/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/neovim/neovim/downloads","issues_url":"https://api.github.com/repos/neovim/neovim/issues{/number}","pulls_url":"https://api.github.com/repos/neovim/neovim/pulls{/number}","milestones_url":"https://api.github.com/repos/neovim/neovim/milestones{/number}","notifications_url":"https://api.github.com/repos/neovim/neovim/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/neovim/neovim/labels{/name}","releases_url":"https://api.github.com/repos/neovim/neovim/releases{/id}","created_at":"2014-01-31T13:39:22Z","updated_at":"2014-05-30T11:55:19Z","pushed_at":"2014-05-29T21:52:26Z","git_url":"git://github.com/neovim/neovim.git","ssh_url":"git@github.com:neovim/neovim.git","clone_url":"https://github.com/neovim/neovim.git","svn_url":"https://github.com/neovim/neovim","homepage":"http://neovim.org/","size":38221,"stargazers_count":8101,"watchers_count":8101,"language":"C","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":502,"mirror_url":null,"open_issues_count":111,"forks":502,"open_issues":111,"watchers":8101,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/neovim/neovim/pulls/783"},"html":{"href":"https://github.com/neovim/neovim/pull/783"},"issue":{"href":"https://api.github.com/repos/neovim/neovim/issues/783"},"comments":{"href":"https://api.github.com/repos/neovim/neovim/issues/783/comments"},"review_comments":{"href":"https://api.github.com/repos/neovim/neovim/pulls/783/comments"},"review_comment":{"href":"https://api.github.com/repos/neovim/neovim/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/neovim/neovim/pulls/783/commits"},"statuses":{"href":"https://api.github.com/repos/neovim/neovim/statuses/b41af754c732da0aea520b725930c7457e9eb5ab"}},"merged":false,"mergeable":null,"mergeable_state":"unknown","merged_by":null,"comments":0,"review_comments":0,"commits":2,"additions":4,"deletions":12,"changed_files":2}}