rocksdb
rocksdb copied to clipboard
facebook
Capture fewer variables to avoid dynamic memory allocation
#10453 uses the memory from stack instead of heap, but the user might save the
callback and invoke that later, if the read is return, it causes ASAN
stack-use-after-return error. So we have to switch back to include captured
local variable to std::function, so at least when the user copy the callback,
it also copies the captured variables. Change it to capture the reference of the
table reader BlockBasedTable
Test Plan: Added benchmark for TableReader.Get(), which shows ~2.5% improvement:
./reader_bench --benchmark_filter=BlockBasedTableGet --benchmark_repetitions=10
# before the change:
BlockBasedTableGet/iterations:1000000_mean 741 ns 741 ns 10
# after the change:
BlockBasedTableGet/iterations:1000000_mean 722 ns 722 ns 10
@jay-zhuang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@jay-zhuang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@jay-zhuang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@jay-zhuang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@jay-zhuang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@jay-zhuang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
By the way, I have a refactoring in progress that should completely solve this issue
By the way, I have a refactoring in progress that should completely solve this issue
I should say my solution also depends on the table reader staying alive, but should be cleaner because of other refactoring. That's the hope.