From 28bf7ba77d7fe1c20370ec1282903dffe90ef3e1 Mon Sep 17 00:00:00 2001 From: rogertyang Date: Tue, 23 May 2023 12:16:24 -0700 Subject: [PATCH] remove unnecessary code in super version getter (#11452) Summary: Do not bother comparing the version of the local super version handle with the global one. An inequality comparison result indicates nothing but a spurious obsoleteness. It only happens when the writer has increased the `ColumnFamilyData::super_version_number_`(https://github.com/facebook/rocksdb/blob/5fc57eec2b44337289f25c1b5687beb54ad709a2/db/column_family.cc#L1309) but has not yet called `ResetThreadLocalSuperVersions()`(https://github.com/facebook/rocksdb/blob/5fc57eec2b44337289f25c1b5687beb54ad709a2/db/column_family.cc#L1328) at the time when a reader reads the local handle(`void* ptr = local_sv_->Swap(SuperVersion::kSVInUse);`). In other words, the existence of a local handle is a sufficent evidence of its fressness. With this PR, we save one or even two atomic instructions when getting a handle of super version. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11452 Reviewed By: ajkr Differential Revision: D46059317 Pulled By: cbi42 fbshipit-source-id: 68b4b1ca8a9929a4aa470105c37a09e0625b014d --- db/column_family.cc | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/db/column_family.cc b/db/column_family.cc index de94426d7..369dc6615 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -1245,30 +1245,11 @@ SuperVersion* ColumnFamilyData::GetThreadLocalSuperVersion(DBImpl* db) { // (if no Scrape happens). assert(ptr != SuperVersion::kSVInUse); SuperVersion* sv = static_cast(ptr); - if (sv == SuperVersion::kSVObsolete || - sv->version_number != super_version_number_.load()) { + if (sv == SuperVersion::kSVObsolete) { RecordTick(ioptions_.stats, NUMBER_SUPERVERSION_ACQUIRES); - SuperVersion* sv_to_delete = nullptr; - - if (sv && sv->Unref()) { - RecordTick(ioptions_.stats, NUMBER_SUPERVERSION_CLEANUPS); - db->mutex()->Lock(); - // NOTE: underlying resources held by superversion (sst files) might - // not be released until the next background job. - sv->Cleanup(); - if (db->immutable_db_options().avoid_unnecessary_blocking_io) { - db->AddSuperVersionsToFreeQueue(sv); - db->SchedulePurge(); - } else { - sv_to_delete = sv; - } - } else { - db->mutex()->Lock(); - } + db->mutex()->Lock(); sv = super_version_->Ref(); db->mutex()->Unlock(); - - delete sv_to_delete; } assert(sv != nullptr); return sv;