Avoid extra mutex for getting blob file reader #311
Conversation
Signed-off-by: Connor1996 <[email protected]>
Signed-off-by: Connor1996 <[email protected]>
Signed-off-by: Connor1996 <[email protected]>
Signed-off-by: Connor1996 <[email protected]>
| std::to_string(index.file_number)); | ||
| return file_cache_->Get(options, sfile->file_number(), sfile->file_size(), | ||
| index.blob_handle, record, value); | ||
| return file_cache_->Get(options, index.file_number, index.blob_handle, record, |
There was a problem hiding this comment.
It will reduce the cost of a lock, but will add a cost to get file size from handle, is it cheaper?
There was a problem hiding this comment.
Agree, can you give some numbers?
There was a problem hiding this comment.
Hard to tell, because it depends on how long the mutex is held by others. As the mutex is shared by multiple fields, it's better to use an operation not affected by the mutex.
At least, rocksdb does this way https://github.com/tikv/rocksdb/blob/6.29.tikv/db/blob/blob_file_reader.cc#L97
There was a problem hiding this comment.
OK. After reading the code again, I think the difference is that before this PR, the lock is accquired for every call.
While for this PR, the getting file size only happens when cache miss. So overall it should be better.
Signed-off-by: Connor1996 <[email protected]>
Should merge #309 first