{"action":"created","comment":{"url":"https://api.github.com/repos/facebook/rocksdb/pulls/comments/88574289","pull_request_review_id":9145271,"id":88574289,"diff_hunk":"@@ -283,19 +285,19 @@ Status TableCache::Get(const ReadOptions& options,\n                        GetContext* get_context, HistogramImpl* file_read_hist,\n                        bool skip_filters, int level) {\n   Status s;\n+  Cache::Handle* handle = nullptr;\n   if (get_context->range_del_agg() != nullptr &&\n       !options.ignore_range_deletions) {","path":"db/table_cache.cc","position":111,"original_position":111,"commit_id":"f2697bfb8da7f19b1c518b520215c9efe6f16a35","original_commit_id":"f2697bfb8da7f19b1c518b520215c9efe6f16a35","user":{"login":"ajkr","id":4780362,"avatar_url":"https://avatars.githubusercontent.com/u/4780362?v=3","gravatar_id":"","url":"https://api.github.com/users/ajkr","html_url":"https://github.com/ajkr","followers_url":"https://api.github.com/users/ajkr/followers","following_url":"https://api.github.com/users/ajkr/following{/other_user}","gists_url":"https://api.github.com/users/ajkr/gists{/gist_id}","starred_url":"https://api.github.com/users/ajkr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ajkr/subscriptions","organizations_url":"https://api.github.com/users/ajkr/orgs","repos_url":"https://api.github.com/users/ajkr/repos","events_url":"https://api.github.com/users/ajkr/events{/privacy}","received_events_url":"https://api.github.com/users/ajkr/received_events","type":"User","site_admin":false},"body":"i'd rather fix the perf regressions if possible because true isn't a sensible default","created_at":"2016-11-17T23:55:32Z","updated_at":"2016-11-17T23:55:32Z","html_url":"https://github.com/facebook/rocksdb/pull/1537#discussion_r88574289","pull_request_url":"https://api.github.com/repos/facebook/rocksdb/pulls/1537","_links":{"self":{"href":"https://api.github.com/repos/facebook/rocksdb/pulls/comments/88574289"},"html":{"href":"https://github.com/facebook/rocksdb/pull/1537#discussion_r88574289"},"pull_request":{"href":"https://api.github.com/repos/facebook/rocksdb/pulls/1537"}}},"pull_request":{"url":"https://api.github.com/repos/facebook/rocksdb/pulls/1537","id":94271056,"html_url":"https://github.com/facebook/rocksdb/pull/1537","diff_url":"https://github.com/facebook/rocksdb/pull/1537.diff","patch_url":"https://github.com/facebook/rocksdb/pull/1537.patch","issue_url":"https://api.github.com/repos/facebook/rocksdb/issues/1537","number":1537,"state":"open","locked":false,"title":"Eliminate redundant cache lookup with range deletion","user":{"login":"ajkr","id":4780362,"avatar_url":"https://avatars.githubusercontent.com/u/4780362?v=3","gravatar_id":"","url":"https://api.github.com/users/ajkr","html_url":"https://github.com/ajkr","followers_url":"https://api.github.com/users/ajkr/followers","following_url":"https://api.github.com/users/ajkr/following{/other_user}","gists_url":"https://api.github.com/users/ajkr/gists{/gist_id}","starred_url":"https://api.github.com/users/ajkr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ajkr/subscriptions","organizations_url":"https://api.github.com/users/ajkr/orgs","repos_url":"https://api.github.com/users/ajkr/repos","events_url":"https://api.github.com/users/ajkr/events{/privacy}","received_events_url":"https://api.github.com/users/ajkr/received_events","type":"User","site_admin":false},"body":"When we introduced range deletion block, TableCache::Get() and TableCache::NewIterator() each did two table cache lookups, one for range deletion block iterator and another for getting the table reader to which the Get()/NewIterator() is delegated. This extra cache lookup was very CPU-intensive (about 10% overhead in a read-heavy benchmark). We can avoid it by reusing the Cache::Handle created for range deletion block iterator to get the file reader.\r\n\r\nTest Plan:\r\n\r\ncommand:\r\n\r\n```\r\nbpl=10485760;overlap=10;mcz=2;del=300000000;levels=6;ctrig=4; delay=8; stop=12; wbn=3; mbc=20; mb=67108864;wbs=134217728; dds=0; sync=0; r=10000000; t=32; vs=800; bs=4096; cs=1048576; of=500000; si=1000000; ./db_bench --benchmarks=readrandom --disable_seek_compaction=1 --mmap_read=0 --statistics=1 --histogram=1 --num=$r --threads=$t --value_size=$vs --block_size=$bs --cache_size=$cs --bloom_bits=10 --cache_numshardbits=6 --open_files=$of --verify_checksum=1 --db=/data/del_range_bench --sync=$sync --disable_wal=1 --compression_type=none --stats_interval=$si --compression_ratio=0.5 --disable_data_sync=$dds --write_buffer_size=$wbs --target_file_size_base=$mb --max_write_buffer_number=$wbn --max_background_compactions=$mbc --level0_file_num_compaction_trigger=$ctrig --level0_slowdown_writes_trigger=$delay --level0_stop_writes_trigger=$stop --num_levels=$levels --delete_obsolete_files_period_micros=$del --min_level_to_compress=$mcz --stats_per_interval=1 --max_bytes_for_level_base=$bpl --use_existing_db=1\r\n```\r\n\r\nbefore:\r\n\r\n`readrandom   :       0.637 micros/op 1569129 ops/sec; 1221.1 MB/s (10000000 of 10000000 found)`\r\n\r\nafter:\r\n\r\n`readrandom   :       0.581 micros/op 1720966 ops/sec; 1339.3 MB/s (10000000 of 10000000 found)`","created_at":"2016-11-17T23:08:32Z","updated_at":"2016-11-17T23:55:32Z","closed_at":null,"merged_at":null,"merge_commit_sha":"49da132dce37b7551a97d9e5d809b29e3adca973","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/facebook/rocksdb/pulls/1537/commits","review_comments_url":"https://api.github.com/repos/facebook/rocksdb/pulls/1537/comments","review_comment_url":"https://api.github.com/repos/facebook/rocksdb/pulls/comments{/number}","comments_url":"https://api.github.com/repos/facebook/rocksdb/issues/1537/comments","statuses_url":"https://api.github.com/repos/facebook/rocksdb/statuses/f2697bfb8da7f19b1c518b520215c9efe6f16a35","head":{"label":"ajkr:lkjxvjoapsdijapoifu","ref":"lkjxvjoapsdijapoifu","sha":"f2697bfb8da7f19b1c518b520215c9efe6f16a35","user":{"login":"ajkr","id":4780362,"avatar_url":"https://avatars.githubusercontent.com/u/4780362?v=3","gravatar_id":"","url":"https://api.github.com/users/ajkr","html_url":"https://github.com/ajkr","followers_url":"https://api.github.com/users/ajkr/followers","following_url":"https://api.github.com/users/ajkr/following{/other_user}","gists_url":"https://api.github.com/users/ajkr/gists{/gist_id}","starred_url":"https://api.github.com/users/ajkr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ajkr/subscriptions","organizations_url":"https://api.github.com/users/ajkr/orgs","repos_url":"https://api.github.com/users/ajkr/repos","events_url":"https://api.github.com/users/ajkr/events{/privacy}","received_events_url":"https://api.github.com/users/ajkr/received_events","type":"User","site_admin":false},"repo":{"id":69681666,"name":"rocksdb","full_name":"ajkr/rocksdb","owner":{"login":"ajkr","id":4780362,"avatar_url":"https://avatars.githubusercontent.com/u/4780362?v=3","gravatar_id":"","url":"https://api.github.com/users/ajkr","html_url":"https://github.com/ajkr","followers_url":"https://api.github.com/users/ajkr/followers","following_url":"https://api.github.com/users/ajkr/following{/other_user}","gists_url":"https://api.github.com/users/ajkr/gists{/gist_id}","starred_url":"https://api.github.com/users/ajkr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ajkr/subscriptions","organizations_url":"https://api.github.com/users/ajkr/orgs","repos_url":"https://api.github.com/users/ajkr/repos","events_url":"https://api.github.com/users/ajkr/events{/privacy}","received_events_url":"https://api.github.com/users/ajkr/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/ajkr/rocksdb","description":"A library that provides an embeddable, persistent key-value store for fast storage.","fork":true,"url":"https://api.github.com/repos/ajkr/rocksdb","forks_url":"https://api.github.com/repos/ajkr/rocksdb/forks","keys_url":"https://api.github.com/repos/ajkr/rocksdb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ajkr/rocksdb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ajkr/rocksdb/teams","hooks_url":"https://api.github.com/repos/ajkr/rocksdb/hooks","issue_events_url":"https://api.github.com/repos/ajkr/rocksdb/issues/events{/number}","events_url":"https://api.github.com/repos/ajkr/rocksdb/events","assignees_url":"https://api.github.com/repos/ajkr/rocksdb/assignees{/user}","branches_url":"https://api.github.com/repos/ajkr/rocksdb/branches{/branch}","tags_url":"https://api.github.com/repos/ajkr/rocksdb/tags","blobs_url":"https://api.github.com/repos/ajkr/rocksdb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ajkr/rocksdb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ajkr/rocksdb/git/refs{/sha}","trees_url":"https://api.github.com/repos/ajkr/rocksdb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ajkr/rocksdb/statuses/{sha}","languages_url":"https://api.github.com/repos/ajkr/rocksdb/languages","stargazers_url":"https://api.github.com/repos/ajkr/rocksdb/stargazers","contributors_url":"https://api.github.com/repos/ajkr/rocksdb/contributors","subscribers_url":"https://api.github.com/repos/ajkr/rocksdb/subscribers","subscription_url":"https://api.github.com/repos/ajkr/rocksdb/subscription","commits_url":"https://api.github.com/repos/ajkr/rocksdb/commits{/sha}","git_commits_url":"https://api.github.com/repos/ajkr/rocksdb/git/commits{/sha}","comments_url":"https://api.github.com/repos/ajkr/rocksdb/comments{/number}","issue_comment_url":"https://api.github.com/repos/ajkr/rocksdb/issues/comments{/number}","contents_url":"https://api.github.com/repos/ajkr/rocksdb/contents/{+path}","compare_url":"https://api.github.com/repos/ajkr/rocksdb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ajkr/rocksdb/merges","archive_url":"https://api.github.com/repos/ajkr/rocksdb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ajkr/rocksdb/downloads","issues_url":"https://api.github.com/repos/ajkr/rocksdb/issues{/number}","pulls_url":"https://api.github.com/repos/ajkr/rocksdb/pulls{/number}","milestones_url":"https://api.github.com/repos/ajkr/rocksdb/milestones{/number}","notifications_url":"https://api.github.com/repos/ajkr/rocksdb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ajkr/rocksdb/labels{/name}","releases_url":"https://api.github.com/repos/ajkr/rocksdb/releases{/id}","deployments_url":"https://api.github.com/repos/ajkr/rocksdb/deployments","created_at":"2016-09-30T16:09:32Z","updated_at":"2016-09-30T16:09:37Z","pushed_at":"2016-11-17T22:48:13Z","git_url":"git://github.com/ajkr/rocksdb.git","ssh_url":"git@github.com:ajkr/rocksdb.git","clone_url":"https://github.com/ajkr/rocksdb.git","svn_url":"https://github.com/ajkr/rocksdb","homepage":"http://rocksdb.org","size":124377,"stargazers_count":0,"watchers_count":0,"language":"C++","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":"facebook:master","ref":"master","sha":"635a7bd1adb9e9d943bebb34dc9e42789a06e25f","user":{"login":"facebook","id":69631,"avatar_url":"https://avatars.githubusercontent.com/u/69631?v=3","gravatar_id":"","url":"https://api.github.com/users/facebook","html_url":"https://github.com/facebook","followers_url":"https://api.github.com/users/facebook/followers","following_url":"https://api.github.com/users/facebook/following{/other_user}","gists_url":"https://api.github.com/users/facebook/gists{/gist_id}","starred_url":"https://api.github.com/users/facebook/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/facebook/subscriptions","organizations_url":"https://api.github.com/users/facebook/orgs","repos_url":"https://api.github.com/users/facebook/repos","events_url":"https://api.github.com/users/facebook/events{/privacy}","received_events_url":"https://api.github.com/users/facebook/received_events","type":"Organization","site_admin":false},"repo":{"id":6934395,"name":"rocksdb","full_name":"facebook/rocksdb","owner":{"login":"facebook","id":69631,"avatar_url":"https://avatars.githubusercontent.com/u/69631?v=3","gravatar_id":"","url":"https://api.github.com/users/facebook","html_url":"https://github.com/facebook","followers_url":"https://api.github.com/users/facebook/followers","following_url":"https://api.github.com/users/facebook/following{/other_user}","gists_url":"https://api.github.com/users/facebook/gists{/gist_id}","starred_url":"https://api.github.com/users/facebook/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/facebook/subscriptions","organizations_url":"https://api.github.com/users/facebook/orgs","repos_url":"https://api.github.com/users/facebook/repos","events_url":"https://api.github.com/users/facebook/events{/privacy}","received_events_url":"https://api.github.com/users/facebook/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/facebook/rocksdb","description":"A library that provides an embeddable, persistent key-value store for fast storage.","fork":false,"url":"https://api.github.com/repos/facebook/rocksdb","forks_url":"https://api.github.com/repos/facebook/rocksdb/forks","keys_url":"https://api.github.com/repos/facebook/rocksdb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/facebook/rocksdb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/facebook/rocksdb/teams","hooks_url":"https://api.github.com/repos/facebook/rocksdb/hooks","issue_events_url":"https://api.github.com/repos/facebook/rocksdb/issues/events{/number}","events_url":"https://api.github.com/repos/facebook/rocksdb/events","assignees_url":"https://api.github.com/repos/facebook/rocksdb/assignees{/user}","branches_url":"https://api.github.com/repos/facebook/rocksdb/branches{/branch}","tags_url":"https://api.github.com/repos/facebook/rocksdb/tags","blobs_url":"https://api.github.com/repos/facebook/rocksdb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/facebook/rocksdb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/facebook/rocksdb/git/refs{/sha}","trees_url":"https://api.github.com/repos/facebook/rocksdb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/facebook/rocksdb/statuses/{sha}","languages_url":"https://api.github.com/repos/facebook/rocksdb/languages","stargazers_url":"https://api.github.com/repos/facebook/rocksdb/stargazers","contributors_url":"https://api.github.com/repos/facebook/rocksdb/contributors","subscribers_url":"https://api.github.com/repos/facebook/rocksdb/subscribers","subscription_url":"https://api.github.com/repos/facebook/rocksdb/subscription","commits_url":"https://api.github.com/repos/facebook/rocksdb/commits{/sha}","git_commits_url":"https://api.github.com/repos/facebook/rocksdb/git/commits{/sha}","comments_url":"https://api.github.com/repos/facebook/rocksdb/comments{/number}","issue_comment_url":"https://api.github.com/repos/facebook/rocksdb/issues/comments{/number}","contents_url":"https://api.github.com/repos/facebook/rocksdb/contents/{+path}","compare_url":"https://api.github.com/repos/facebook/rocksdb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/facebook/rocksdb/merges","archive_url":"https://api.github.com/repos/facebook/rocksdb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/facebook/rocksdb/downloads","issues_url":"https://api.github.com/repos/facebook/rocksdb/issues{/number}","pulls_url":"https://api.github.com/repos/facebook/rocksdb/pulls{/number}","milestones_url":"https://api.github.com/repos/facebook/rocksdb/milestones{/number}","notifications_url":"https://api.github.com/repos/facebook/rocksdb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/facebook/rocksdb/labels{/name}","releases_url":"https://api.github.com/repos/facebook/rocksdb/releases{/id}","deployments_url":"https://api.github.com/repos/facebook/rocksdb/deployments","created_at":"2012-11-30T06:16:18Z","updated_at":"2016-11-17T20:18:18Z","pushed_at":"2016-11-17T23:08:32Z","git_url":"git://github.com/facebook/rocksdb.git","ssh_url":"git@github.com:facebook/rocksdb.git","clone_url":"https://github.com/facebook/rocksdb.git","svn_url":"https://github.com/facebook/rocksdb","homepage":"http://rocksdb.org","size":131341,"stargazers_count":6441,"watchers_count":6441,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":1349,"mirror_url":null,"open_issues_count":306,"forks":1349,"open_issues":306,"watchers":6441,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/facebook/rocksdb/pulls/1537"},"html":{"href":"https://github.com/facebook/rocksdb/pull/1537"},"issue":{"href":"https://api.github.com/repos/facebook/rocksdb/issues/1537"},"comments":{"href":"https://api.github.com/repos/facebook/rocksdb/issues/1537/comments"},"review_comments":{"href":"https://api.github.com/repos/facebook/rocksdb/pulls/1537/comments"},"review_comment":{"href":"https://api.github.com/repos/facebook/rocksdb/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/facebook/rocksdb/pulls/1537/commits"},"statuses":{"href":"https://api.github.com/repos/facebook/rocksdb/statuses/f2697bfb8da7f19b1c518b520215c9efe6f16a35"}}}}
{"actor":{"display_login":"ajkr"}}