{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/4206","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/4206/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/4206/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/4206/events","html_url":"https://github.com/cockroachdb/cockroach/pull/4206","id":132001419,"number":4206,"title":"Reduce allocations in IntervalCache and its usage.","user":{"login":"petermattis","id":6201335,"avatar_url":"https://avatars.githubusercontent.com/u/6201335?v=3","gravatar_id":"","url":"https://api.github.com/users/petermattis","html_url":"https://github.com/petermattis","followers_url":"https://api.github.com/users/petermattis/followers","following_url":"https://api.github.com/users/petermattis/following{/other_user}","gists_url":"https://api.github.com/users/petermattis/gists{/gist_id}","starred_url":"https://api.github.com/users/petermattis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/petermattis/subscriptions","organizations_url":"https://api.github.com/users/petermattis/orgs","repos_url":"https://api.github.com/users/petermattis/repos","events_url":"https://api.github.com/users/petermattis/events{/privacy}","received_events_url":"https://api.github.com/users/petermattis/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2016-02-07T19:29:12Z","updated_at":"2016-02-07T20:54:08Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/4206","html_url":"https://github.com/cockroachdb/cockroach/pull/4206","diff_url":"https://github.com/cockroachdb/cockroach/pull/4206.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/4206.patch"},"body":"IntervalCache is the data structure backing TimestampCache, the\ntransaction coordinator's key cache and the storage layer's\nCommandQueue. Reducing allocations here has an outsized impact.\n\nThere is further work to be done here. All uses of IntervalCache are\nusing roachpb.{Key,RKey} as the key. But storing those keys in an\ninterval.Comparable interface requires an allocation (a []byte is\nessentially a struct with 3 fields). Given how frequently used\nIntervalCache is, we may want to fork the underlying\nbiogo/store/interval.Tree and specialize it for []byte.\n\n```\nname                   old time/op    new time/op    delta\nInsert1_Cockroach-8       353µs ± 1%     349µs ± 1%   -1.00%  (p=0.000 n=10+10)\nInsert10_Cockroach-8      731µs ± 3%     715µs ± 2%   -2.15%  (p=0.002 n=10+10)\nInsert100_Cockroach-8    4.04ms ± 2%    3.95ms ± 1%   -2.36%  (p=0.000 n=10+10)\nUpdate1_Cockroach-8       811µs ± 1%     785µs ± 1%   -3.24%    (p=0.000 n=9+9)\nUpdate10_Cockroach-8     4.38ms ± 1%    4.12ms ± 2%   -5.95%  (p=0.000 n=10+10)\nUpdate100_Cockroach-8    39.6ms ± 1%    36.7ms ± 1%   -7.10%  (p=0.000 n=10+10)\nDelete1_Cockroach-8      1.14ms ± 1%    1.11ms ± 1%   -2.56%    (p=0.000 n=9+9)\nDelete10_Cockroach-8     1.86ms ± 1%    1.67ms ± 1%  -10.29%   (p=0.000 n=10+8)\nDelete100_Cockroach-8    10.7ms ± 2%     8.2ms ± 1%  -23.41%   (p=0.000 n=10+9)\nScan1_Cockroach-8         169µs ± 1%     159µs ± 1%   -5.81%   (p=0.000 n=10+9)\nScan10_Cockroach-8        208µs ± 1%     191µs ± 1%   -8.40%    (p=0.000 n=9+8)\nScan100_Cockroach-8       573µs ± 4%     501µs ± 5%  -12.62%  (p=0.000 n=10+10)\n\nname                   old allocs/op  new allocs/op  delta\nInsert1_Cockroach-8         382 ± 0%       361 ± 0%   -5.42%  (p=0.000 n=10+10)\nInsert10_Cockroach-8      1.02k ± 0%     0.94k ± 0%   -8.44%  (p=0.000 n=10+10)\nInsert100_Cockroach-8     7.21k ± 0%     6.48k ± 0%  -10.17%   (p=0.000 n=10+9)\nUpdate1_Cockroach-8         882 ± 0%       831 ± 0%   -5.78%  (p=0.000 n=10+10)\nUpdate10_Cockroach-8      6.13k ± 0%     5.72k ± 0%   -6.65%   (p=0.000 n=7+10)\nUpdate100_Cockroach-8     58.5k ± 0%     54.5k ± 0%   -6.84%    (p=0.000 n=9+9)\nDelete1_Cockroach-8         838 ± 0%       772 ± 0%   -7.95%  (p=0.000 n=10+10)\nDelete10_Cockroach-8      2.31k ± 0%     1.75k ± 0%  -24.36%   (p=0.000 n=8+10)\nDelete100_Cockroach-8     16.4k ± 0%     11.0k ± 0%  -33.07%    (p=0.000 n=9+9)\nScan1_Cockroach-8           227 ± 0%       208 ± 0%   -8.34%  (p=0.000 n=10+10)\nScan10_Cockroach-8          317 ± 0%       287 ± 0%   -9.25%  (p=0.000 n=10+10)\nScan100_Cockroach-8       1.05k ± 0%     1.01k ± 0%   -3.61%  (p=0.000 n=10+10)\n```\n\n<!-- Reviewable:start -->\n[<img src=\"https://reviewable.io/review_button.svg\" height=\"40\" alt=\"Review on Reviewable\"/>](https://reviewable.io/reviews/cockroachdb/cockroach/4206)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/181114606","html_url":"https://github.com/cockroachdb/cockroach/pull/4206#issuecomment-181114606","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/4206","id":181114606,"user":{"login":"petermattis","id":6201335,"avatar_url":"https://avatars.githubusercontent.com/u/6201335?v=3","gravatar_id":"","url":"https://api.github.com/users/petermattis","html_url":"https://github.com/petermattis","followers_url":"https://api.github.com/users/petermattis/followers","following_url":"https://api.github.com/users/petermattis/following{/other_user}","gists_url":"https://api.github.com/users/petermattis/gists{/gist_id}","starred_url":"https://api.github.com/users/petermattis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/petermattis/subscriptions","organizations_url":"https://api.github.com/users/petermattis/orgs","repos_url":"https://api.github.com/users/petermattis/repos","events_url":"https://api.github.com/users/petermattis/events{/privacy}","received_events_url":"https://api.github.com/users/petermattis/received_events","type":"User","site_admin":false},"created_at":"2016-02-07T20:54:08Z","updated_at":"2016-02-07T20:54:08Z","body":"\n\n\n\nReview status: all files reviewed at latest revision, 5 unresolved discussions.\n\n---\n\n*[kv/txn_coord_sender.go, line 105 \\[r1\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/4206#-K9xFvqDr9GPOE4jSokK:-K9xU8kjuzZiddC4I9cY:-1672423218) ([raw file](https://github.com/cockroachdb/cockroach/blob/020009310933d7b070fa839c83f714f7dab94b7f/kv/txn_coord_sender.go#L105)):*\nYes, it's subtle. The arguments to `GetOverlaps` are `interval.Comparable`. Assigning a `roachpb.Key` (i.e. a `[]byte`) to an interface requires an allocation. But we've already made such an assignment on the previous line when we created the `cache.IntervalKey`. Using `key.{Start,End}()`saves 2 allocations on every call to `addKeyRange`.\n\n---\n\n*[kv/txn_coord_sender.go, line 116 \\[r1\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/4206#-K9xG7dNVadJd-8wihYE:-K9xUVdO8jxdjcngewyh:-2012997982) ([raw file](https://github.com/cockroachdb/cockroach/blob/020009310933d7b070fa839c83f714f7dab94b7f/kv/txn_coord_sender.go#L116)):*\nI doubt there would be any measurable efficiency change, but might was well let the compiler do the heap allocation. \n\n---\n\n*[storage/timestamp_cache.go, line 105 \\[r1\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/4206#-K9xGX3OLnHjORAeKuaN:-K9xVU9UC-KorxPHgMRm:-722688495) ([raw file](https://github.com/cockroachdb/cockroach/blob/020009310933d7b070fa839c83f714f7dab94b7f/storage/timestamp_cache.go#L105)):*\nSee other comment.\n\n---\n\n*[storage/timestamp_cache.go, line 124 \\[r1\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/4206#-K9xGwWn60oFt8hHJG4N:-K9xVUvbyxqOeELUPq9m:-76302329) ([raw file](https://github.com/cockroachdb/cockroach/blob/020009310933d7b070fa839c83f714f7dab94b7f/storage/timestamp_cache.go#L124)):*\nI've changed this to allocate the key and entry together.\n\n---\n\n*[util/cache/cache.go, line 412 \\[r1\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/4206#-K9xFb4mFzinn0RAIc0L:-K9xVfs9EQPRRtgAYjFF:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/020009310933d7b070fa839c83f714f7dab94b7f/util/cache/cache.go#L412)):*\nDone.\n\n---\n\n\n*Comments from the [review on Reviewable.io](https://reviewable.io:443/reviews/cockroachdb/cockroach/4206)*\n<!-- Sent from Reviewable.io -->\n"}}