{"action":"created","issue":{"url":"https://api.github.com/repos/coreos/etcd/issues/3588","labels_url":"https://api.github.com/repos/coreos/etcd/issues/3588/labels{/name}","comments_url":"https://api.github.com/repos/coreos/etcd/issues/3588/comments","events_url":"https://api.github.com/repos/coreos/etcd/issues/3588/events","html_url":"https://github.com/coreos/etcd/pull/3588","id":108105333,"number":3588,"title":"storage/watchable_store.go: use map for unsynced","user":{"login":"gyuho","id":6799218,"avatar_url":"https://avatars.githubusercontent.com/u/6799218?v=3","gravatar_id":"","url":"https://api.github.com/users/gyuho","html_url":"https://github.com/gyuho","followers_url":"https://api.github.com/users/gyuho/followers","following_url":"https://api.github.com/users/gyuho/following{/other_user}","gists_url":"https://api.github.com/users/gyuho/gists{/gist_id}","starred_url":"https://api.github.com/users/gyuho/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gyuho/subscriptions","organizations_url":"https://api.github.com/users/gyuho/orgs","repos_url":"https://api.github.com/users/gyuho/repos","events_url":"https://api.github.com/users/gyuho/events{/privacy}","received_events_url":"https://api.github.com/users/gyuho/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":28,"created_at":"2015-09-24T10:48:01Z","updated_at":"2015-10-21T23:50:00Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/coreos/etcd/pulls/3588","html_url":"https://github.com/coreos/etcd/pull/3588","diff_url":"https://github.com/coreos/etcd/pull/3588.diff","patch_url":"https://github.com/coreos/etcd/pull/3588.patch"},"body":"This is a new pull request following up with:\r\n- https://github.com/coreos/etcd/pull/3575\r\n- https://github.com/coreos/etcd/pull/3582\r\n\r\nAnd the goal of this PR is, as @yichengq suggests, `use map to reduce cancel cost,\r\nand benchmark its difference`. As you see in my [example benchmark](https://github.com/gyuho/learn/blob/master/doc/go_function_method_pointer_nil_map_slice/slice_vs_map/benchmark_results.txt) [1], theoretically Go map should be faster in\r\ndeletion. And current watchable store does deletion in its unsynced slice with\r\ncancel function. So I expected map version to be faster, if not slower:\r\n\r\n```\r\ngo test -bench='BenchmarkWatchableStoreUnsynced' -benchmem -cpu 1,2,4,8 > slice.txt;\r\n\r\ngo test -bench='BenchmarkWatchableStoreUnsynced' -benchmem -cpu 1,2,4,8 > map.txt;\r\n\r\ngo get -v -u golang.org/x/tools/cmd/benchcmp;\r\nbenchcmp slice.txt map.txt > benchmark_results.txt;\r\n```\r\n\r\n<br>\r\nResults (old is original code with slice, **negative** means `map` version is **faster** than `slice` version):\r\n\r\n```\r\nbenchmark                              old ns/op     new ns/op     delta\r\nBenchmarkWatchableStoreUnsynced        15358884      8238657       -46.36%\r\nBenchmarkWatchableStoreUnsynced-2      2672428       2701123       +1.07%\r\nBenchmarkWatchableStoreUnsynced-4      2253534       2445470       +8.52%\r\nBenchmarkWatchableStoreUnsynced-8      2139670       2103291       -1.70%\r\n\r\nbenchmark                              old allocs     new allocs     delta\r\nBenchmarkWatchableStoreUnsynced        10912          10330          -5.33%\r\nBenchmarkWatchableStoreUnsynced-2      9717           9843           +1.30%\r\nBenchmarkWatchableStoreUnsynced-4      10273          10241          -0.31%\r\nBenchmarkWatchableStoreUnsynced-8      9889           7906           -20.05%\r\n\r\nbenchmark                              old bytes     new bytes     delta\r\nBenchmarkWatchableStoreUnsynced        404238        386040        -4.50%\r\nBenchmarkWatchableStoreUnsynced-2      356543        366115        +2.68%\r\nBenchmarkWatchableStoreUnsynced-4      373095        382577        +2.54%\r\nBenchmarkWatchableStoreUnsynced-8      359913        299902        -16.67%\r\n\r\n```\r\n\r\nSo, my question is:\r\n\r\n1. Do you think my benchmark is valid to compare `slice` and `map` in\r\n   watchable store?\r\n2. If not, what would be the best way to test real-life cases for `unsynced`?\r\n\r\nThanks! And please let me know if you have any feedback.\r\n\r\n---\r\n\r\n1. https://github.com/gyuho/learn/blob/master/doc/go_function_method_pointer_nil_map_slice/slice_vs_map/benchmark_results.txt"},"comment":{"url":"https://api.github.com/repos/coreos/etcd/issues/comments/150056234","html_url":"https://github.com/coreos/etcd/pull/3588#issuecomment-150056234","issue_url":"https://api.github.com/repos/coreos/etcd/issues/3588","id":150056234,"user":{"login":"yichengq","id":2976232,"avatar_url":"https://avatars.githubusercontent.com/u/2976232?v=3","gravatar_id":"","url":"https://api.github.com/users/yichengq","html_url":"https://github.com/yichengq","followers_url":"https://api.github.com/users/yichengq/followers","following_url":"https://api.github.com/users/yichengq/following{/other_user}","gists_url":"https://api.github.com/users/yichengq/gists{/gist_id}","starred_url":"https://api.github.com/users/yichengq/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yichengq/subscriptions","organizations_url":"https://api.github.com/users/yichengq/orgs","repos_url":"https://api.github.com/users/yichengq/repos","events_url":"https://api.github.com/users/yichengq/events{/privacy}","received_events_url":"https://api.github.com/users/yichengq/received_events","type":"User","site_admin":false},"created_at":"2015-10-21T23:49:59Z","updated_at":"2015-10-21T23:49:59Z","body":":ship: Thanks for the contribution!"}}