{"action":"created","comment":{"url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/comments/196067967","pull_request_review_id":129550025,"id":196067967,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDE5NjA2Nzk2Nw==","diff_hunk":"@@ -122,9 +122,10 @@ func (c *Collation) CalculateChunkRoot() {\n // some salt, which is appended to each chunk in the collation body before it\n // is hashed.\n func (c *Collation) CalculatePOC(salt []byte) common.Hash {\n-\tbody := make([]byte, len(c.body))\n+\tbody := make([]byte, 0, len(c.body)*(1+len(salt)))\n+\n \tfor _, chunk := range c.body {\n-\t\tbody = append(body, append(salt, chunk)...) // add salt to each chunk.\n+\t\tbody = append(append(body, salt...), chunk)","path":"sharding/collation.go","position":9,"original_position":9,"commit_id":"874819d40a1f59093fef102edbba1a43b8570a4d","original_commit_id":"874819d40a1f59093fef102edbba1a43b8570a4d","user":{"login":"terenc3t","id":21316537,"node_id":"MDQ6VXNlcjIxMzE2NTM3","avatar_url":"https://avatars1.githubusercontent.com/u/21316537?v=4","gravatar_id":"","url":"https://api.github.com/users/terenc3t","html_url":"https://github.com/terenc3t","followers_url":"https://api.github.com/users/terenc3t/followers","following_url":"https://api.github.com/users/terenc3t/following{/other_user}","gists_url":"https://api.github.com/users/terenc3t/gists{/gist_id}","starred_url":"https://api.github.com/users/terenc3t/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/terenc3t/subscriptions","organizations_url":"https://api.github.com/users/terenc3t/orgs","repos_url":"https://api.github.com/users/terenc3t/repos","events_url":"https://api.github.com/users/terenc3t/events{/privacy}","received_events_url":"https://api.github.com/users/terenc3t/received_events","type":"User","site_admin":false},"body":"Can you explain the logic behind `append(append(body, salt...), chunk)`? Is each chunk 32 byte?","created_at":"2018-06-18T13:00:57Z","updated_at":"2018-06-18T13:01:38Z","html_url":"https://github.com/prysmaticlabs/geth-sharding/pull/182#discussion_r196067967","pull_request_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/182","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/comments/196067967"},"html":{"href":"https://github.com/prysmaticlabs/geth-sharding/pull/182#discussion_r196067967"},"pull_request":{"href":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/182"}}},"pull_request":{"url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/182","id":194777183,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk0Nzc3MTgz","html_url":"https://github.com/prysmaticlabs/geth-sharding/pull/182","diff_url":"https://github.com/prysmaticlabs/geth-sharding/pull/182.diff","patch_url":"https://github.com/prysmaticlabs/geth-sharding/pull/182.patch","issue_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/issues/182","number":182,"state":"open","locked":false,"title":"perf improvements to CalculatePOC","user":{"login":"rawfalafel","id":202157,"node_id":"MDQ6VXNlcjIwMjE1Nw==","avatar_url":"https://avatars0.githubusercontent.com/u/202157?v=4","gravatar_id":"","url":"https://api.github.com/users/rawfalafel","html_url":"https://github.com/rawfalafel","followers_url":"https://api.github.com/users/rawfalafel/followers","following_url":"https://api.github.com/users/rawfalafel/following{/other_user}","gists_url":"https://api.github.com/users/rawfalafel/gists{/gist_id}","starred_url":"https://api.github.com/users/rawfalafel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rawfalafel/subscriptions","organizations_url":"https://api.github.com/users/rawfalafel/orgs","repos_url":"https://api.github.com/users/rawfalafel/repos","events_url":"https://api.github.com/users/rawfalafel/events{/privacy}","received_events_url":"https://api.github.com/users/rawfalafel/received_events","type":"User","site_admin":false},"body":"Old code:\r\n```\r\ngoos: linux\r\ngoarch: amd64\r\npkg: github.com/ethereum/go-ethereum/sharding\r\nBenchmarkCalculatePOC-12             100          11704937 ns/op        14080345 B/op     121660 allocs/op\r\nPASS\r\n```\r\n\r\nNew code:\r\n```\r\ngoos: linux\r\ngoarch: amd64\r\npkg: github.com/ethereum/go-ethereum/sharding\r\nBenchmarkCalculatePOC-12             100          10932186 ns/op        13344667 B/op     115783 allocs/op\r\nPASS\r\n```\r\n\r\nThe change is modest because most of the time in the function is spent merklizing the tree (calling `DeriveSha`). Commenting out the call and running just the construction of the collation body with the salt makes the change more drastic.\r\n\r\nOld code:\r\n```\r\ngoos: linux\r\ngoarch: amd64\r\npkg: github.com/ethereum/go-ethereum/sharding\r\nBenchmarkCalculatePOC-12          100000             11543 ns/op           40064 B/op        309 allocs/op\r\nPASS\r\n```\r\n\r\nNew code:\r\n```\r\ngoos: linux\r\ngoarch: amd64\r\npkg: github.com/ethereum/go-ethereum/sharding\r\nBenchmarkCalculatePOC-12         1000000              1727 ns/op            6528 B/op          1 allocs/op\r\nPASS\r\n```\r\n\r\nNotice that there's only 1 alloc with the new code! That's because all the bytes are allocated at the start and we avoid unnecessarily copying data around.","created_at":"2018-06-14T04:43:18Z","updated_at":"2018-06-18T13:01:38Z","closed_at":null,"merged_at":null,"merge_commit_sha":"a1a94d471c90b86bcc56580b47d5f28933ff2413","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/182/commits","review_comments_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/182/comments","review_comment_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/comments{/number}","comments_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/issues/182/comments","statuses_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/statuses/874819d40a1f59093fef102edbba1a43b8570a4d","head":{"label":"rawfalafel:poc-improvements","ref":"poc-improvements","sha":"874819d40a1f59093fef102edbba1a43b8570a4d","user":{"login":"rawfalafel","id":202157,"node_id":"MDQ6VXNlcjIwMjE1Nw==","avatar_url":"https://avatars0.githubusercontent.com/u/202157?v=4","gravatar_id":"","url":"https://api.github.com/users/rawfalafel","html_url":"https://github.com/rawfalafel","followers_url":"https://api.github.com/users/rawfalafel/followers","following_url":"https://api.github.com/users/rawfalafel/following{/other_user}","gists_url":"https://api.github.com/users/rawfalafel/gists{/gist_id}","starred_url":"https://api.github.com/users/rawfalafel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rawfalafel/subscriptions","organizations_url":"https://api.github.com/users/rawfalafel/orgs","repos_url":"https://api.github.com/users/rawfalafel/repos","events_url":"https://api.github.com/users/rawfalafel/events{/privacy}","received_events_url":"https://api.github.com/users/rawfalafel/received_events","type":"User","site_admin":false},"repo":{"id":135266127,"node_id":"MDEwOlJlcG9zaXRvcnkxMzUyNjYxMjc=","name":"geth-sharding","full_name":"rawfalafel/geth-sharding","owner":{"login":"rawfalafel","id":202157,"node_id":"MDQ6VXNlcjIwMjE1Nw==","avatar_url":"https://avatars0.githubusercontent.com/u/202157?v=4","gravatar_id":"","url":"https://api.github.com/users/rawfalafel","html_url":"https://github.com/rawfalafel","followers_url":"https://api.github.com/users/rawfalafel/followers","following_url":"https://api.github.com/users/rawfalafel/following{/other_user}","gists_url":"https://api.github.com/users/rawfalafel/gists{/gist_id}","starred_url":"https://api.github.com/users/rawfalafel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rawfalafel/subscriptions","organizations_url":"https://api.github.com/users/rawfalafel/orgs","repos_url":"https://api.github.com/users/rawfalafel/repos","events_url":"https://api.github.com/users/rawfalafel/events{/privacy}","received_events_url":"https://api.github.com/users/rawfalafel/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/rawfalafel/geth-sharding","description":"Go implementation of the Ethereum sharding specification and stateless client","fork":true,"url":"https://api.github.com/repos/rawfalafel/geth-sharding","forks_url":"https://api.github.com/repos/rawfalafel/geth-sharding/forks","keys_url":"https://api.github.com/repos/rawfalafel/geth-sharding/keys{/key_id}","collaborators_url":"https://api.github.com/repos/rawfalafel/geth-sharding/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/rawfalafel/geth-sharding/teams","hooks_url":"https://api.github.com/repos/rawfalafel/geth-sharding/hooks","issue_events_url":"https://api.github.com/repos/rawfalafel/geth-sharding/issues/events{/number}","events_url":"https://api.github.com/repos/rawfalafel/geth-sharding/events","assignees_url":"https://api.github.com/repos/rawfalafel/geth-sharding/assignees{/user}","branches_url":"https://api.github.com/repos/rawfalafel/geth-sharding/branches{/branch}","tags_url":"https://api.github.com/repos/rawfalafel/geth-sharding/tags","blobs_url":"https://api.github.com/repos/rawfalafel/geth-sharding/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/rawfalafel/geth-sharding/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/rawfalafel/geth-sharding/git/refs{/sha}","trees_url":"https://api.github.com/repos/rawfalafel/geth-sharding/git/trees{/sha}","statuses_url":"https://api.github.com/repos/rawfalafel/geth-sharding/statuses/{sha}","languages_url":"https://api.github.com/repos/rawfalafel/geth-sharding/languages","stargazers_url":"https://api.github.com/repos/rawfalafel/geth-sharding/stargazers","contributors_url":"https://api.github.com/repos/rawfalafel/geth-sharding/contributors","subscribers_url":"https://api.github.com/repos/rawfalafel/geth-sharding/subscribers","subscription_url":"https://api.github.com/repos/rawfalafel/geth-sharding/subscription","commits_url":"https://api.github.com/repos/rawfalafel/geth-sharding/commits{/sha}","git_commits_url":"https://api.github.com/repos/rawfalafel/geth-sharding/git/commits{/sha}","comments_url":"https://api.github.com/repos/rawfalafel/geth-sharding/comments{/number}","issue_comment_url":"https://api.github.com/repos/rawfalafel/geth-sharding/issues/comments{/number}","contents_url":"https://api.github.com/repos/rawfalafel/geth-sharding/contents/{+path}","compare_url":"https://api.github.com/repos/rawfalafel/geth-sharding/compare/{base}...{head}","merges_url":"https://api.github.com/repos/rawfalafel/geth-sharding/merges","archive_url":"https://api.github.com/repos/rawfalafel/geth-sharding/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/rawfalafel/geth-sharding/downloads","issues_url":"https://api.github.com/repos/rawfalafel/geth-sharding/issues{/number}","pulls_url":"https://api.github.com/repos/rawfalafel/geth-sharding/pulls{/number}","milestones_url":"https://api.github.com/repos/rawfalafel/geth-sharding/milestones{/number}","notifications_url":"https://api.github.com/repos/rawfalafel/geth-sharding/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/rawfalafel/geth-sharding/labels{/name}","releases_url":"https://api.github.com/repos/rawfalafel/geth-sharding/releases{/id}","deployments_url":"https://api.github.com/repos/rawfalafel/geth-sharding/deployments","created_at":"2018-05-29T08:35:36Z","updated_at":"2018-05-29T08:35:43Z","pushed_at":"2018-06-18T05:35:20Z","git_url":"git://github.com/rawfalafel/geth-sharding.git","ssh_url":"git@github.com:rawfalafel/geth-sharding.git","clone_url":"https://github.com/rawfalafel/geth-sharding.git","svn_url":"https://github.com/rawfalafel/geth-sharding","homepage":"https://prysmaticlabs.com/","size":98891,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"lgpl-3.0","name":"GNU Lesser General Public License v3.0","spdx_id":"LGPL-3.0","url":"https://api.github.com/licenses/lgpl-3.0","node_id":"MDc6TGljZW5zZTEy"},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"prysmaticlabs:master","ref":"master","sha":"0c8aac1869fec4122f77ce1cd9e4639368ccabaa","user":{"login":"prysmaticlabs","id":35349758,"node_id":"MDEyOk9yZ2FuaXphdGlvbjM1MzQ5NzU4","avatar_url":"https://avatars2.githubusercontent.com/u/35349758?v=4","gravatar_id":"","url":"https://api.github.com/users/prysmaticlabs","html_url":"https://github.com/prysmaticlabs","followers_url":"https://api.github.com/users/prysmaticlabs/followers","following_url":"https://api.github.com/users/prysmaticlabs/following{/other_user}","gists_url":"https://api.github.com/users/prysmaticlabs/gists{/gist_id}","starred_url":"https://api.github.com/users/prysmaticlabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/prysmaticlabs/subscriptions","organizations_url":"https://api.github.com/users/prysmaticlabs/orgs","repos_url":"https://api.github.com/users/prysmaticlabs/repos","events_url":"https://api.github.com/users/prysmaticlabs/events{/privacy}","received_events_url":"https://api.github.com/users/prysmaticlabs/received_events","type":"Organization","site_admin":false},"repo":{"id":117156031,"node_id":"MDEwOlJlcG9zaXRvcnkxMTcxNTYwMzE=","name":"geth-sharding","full_name":"prysmaticlabs/geth-sharding","owner":{"login":"prysmaticlabs","id":35349758,"node_id":"MDEyOk9yZ2FuaXphdGlvbjM1MzQ5NzU4","avatar_url":"https://avatars2.githubusercontent.com/u/35349758?v=4","gravatar_id":"","url":"https://api.github.com/users/prysmaticlabs","html_url":"https://github.com/prysmaticlabs","followers_url":"https://api.github.com/users/prysmaticlabs/followers","following_url":"https://api.github.com/users/prysmaticlabs/following{/other_user}","gists_url":"https://api.github.com/users/prysmaticlabs/gists{/gist_id}","starred_url":"https://api.github.com/users/prysmaticlabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/prysmaticlabs/subscriptions","organizations_url":"https://api.github.com/users/prysmaticlabs/orgs","repos_url":"https://api.github.com/users/prysmaticlabs/repos","events_url":"https://api.github.com/users/prysmaticlabs/events{/privacy}","received_events_url":"https://api.github.com/users/prysmaticlabs/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/prysmaticlabs/geth-sharding","description":"Go implementation of the Ethereum sharding specification and stateless client","fork":true,"url":"https://api.github.com/repos/prysmaticlabs/geth-sharding","forks_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/forks","keys_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/keys{/key_id}","collaborators_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/teams","hooks_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/hooks","issue_events_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/issues/events{/number}","events_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/events","assignees_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/assignees{/user}","branches_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/branches{/branch}","tags_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/tags","blobs_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/git/refs{/sha}","trees_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/git/trees{/sha}","statuses_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/statuses/{sha}","languages_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/languages","stargazers_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/stargazers","contributors_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/contributors","subscribers_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/subscribers","subscription_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/subscription","commits_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/commits{/sha}","git_commits_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/git/commits{/sha}","comments_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/comments{/number}","issue_comment_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/issues/comments{/number}","contents_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/contents/{+path}","compare_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/compare/{base}...{head}","merges_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/merges","archive_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/downloads","issues_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/issues{/number}","pulls_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls{/number}","milestones_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/milestones{/number}","notifications_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/labels{/name}","releases_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/releases{/id}","deployments_url":"https://api.github.com/repos/prysmaticlabs/geth-sharding/deployments","created_at":"2018-01-11T21:31:33Z","updated_at":"2018-06-17T21:17:05Z","pushed_at":"2018-06-18T11:22:36Z","git_url":"git://github.com/prysmaticlabs/geth-sharding.git","ssh_url":"git@github.com:prysmaticlabs/geth-sharding.git","clone_url":"https://github.com/prysmaticlabs/geth-sharding.git","svn_url":"https://github.com/prysmaticlabs/geth-sharding","homepage":"https://prysmaticlabs.com/","size":98872,"stargazers_count":101,"watchers_count":101,"language":"Go","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":12,"mirror_url":null,"archived":false,"open_issues_count":29,"license":{"key":"lgpl-3.0","name":"GNU Lesser General Public License v3.0","spdx_id":"LGPL-3.0","url":"https://api.github.com/licenses/lgpl-3.0","node_id":"MDc6TGljZW5zZTEy"},"forks":12,"open_issues":29,"watchers":101,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/182"},"html":{"href":"https://github.com/prysmaticlabs/geth-sharding/pull/182"},"issue":{"href":"https://api.github.com/repos/prysmaticlabs/geth-sharding/issues/182"},"comments":{"href":"https://api.github.com/repos/prysmaticlabs/geth-sharding/issues/182/comments"},"review_comments":{"href":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/182/comments"},"review_comment":{"href":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/prysmaticlabs/geth-sharding/pulls/182/commits"},"statuses":{"href":"https://api.github.com/repos/prysmaticlabs/geth-sharding/statuses/874819d40a1f59093fef102edbba1a43b8570a4d"}},"author_association":"NONE"}}
{"actor":{"display_login":"terenc3t"}}