-
Notifications
You must be signed in to change notification settings - Fork 6.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduce IndexBlockIter size #4358
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maysamyabandeh has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Unit test failure seems to be related. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The approach looks good to me. The unit test failure seems to be related though.
table/block.h
Outdated
@@ -468,10 +468,10 @@ class IndexBlockIter final : public BlockIter<BlockHandle> { | |||
BlockPrefixIndex* prefix_index, bool key_includes_seq, | |||
bool value_is_full, bool block_contents_pinned, | |||
DataBlockHashIndex* /*data_block_hash_index*/) { | |||
InitializeBase(comparator, data, restarts, num_restarts, | |||
kDisableGlobalSequenceNumber, block_contents_pinned); | |||
InitializeBase(key_includes_seq_ ? comparator : user_comparator, data, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
key_includes_seq_
has not been initialized yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maysamyabandeh has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: With #3983 the size of IndexBlockIter was increased. This had resulted in a regression on P50 latencies in one of our benchmarks. The patch reduces IndexBlockIter size be eliminating active_comparator_ field from the class. Pull Request resolved: #4358 Differential Revision: D9781737 Pulled By: maysamyabandeh fbshipit-source-id: 71e2b28d90ff0813db9e04b737ae73e185583c52
Summary: With facebook#3983 the size of IndexBlockIter was increased. This had resulted in a regression on P50 latencies in one of our benchmarks. The patch reduces IndexBlockIter size be eliminating active_comparator_ field from the class. Pull Request resolved: facebook#4358 Differential Revision: D9781737 Pulled By: maysamyabandeh fbshipit-source-id: 71e2b28d90ff0813db9e04b737ae73e185583c52
With #3983 the size of IndexBlockIter was increased. This had resulted in a regression on P50 latencies in one of our benchmarks. The patch reduces IndexBlockIter size be eliminating active_comparator_ field from the class.