From caa1fd0e0e26b520ff4fd2da25224a3372935a7d Mon Sep 17 00:00:00 2001 From: Robert Date: Sun, 4 Jan 2015 12:02:52 +0800 Subject: [PATCH 1/3] Improve performance when loading BackupMeta. * Use strtoul() and strtoull() instead of sscanf(). glibc's sscanf() will do a implicit strlen(). * Move implicit construction of Slice("crc32 ") out of loop. --- utilities/backupable/backupable_db.cc | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/utilities/backupable/backupable_db.cc b/utilities/backupable/backupable_db.cc index 269e9e9f1..ca7521ff3 100644 --- a/utilities/backupable/backupable_db.cc +++ b/utilities/backupable/backupable_db.cc @@ -20,6 +20,7 @@ #endif #include +#include #include #include #include @@ -1163,16 +1164,18 @@ Status BackupEngineImpl::BackupMeta::LoadFromFile( buf[data.size()] = 0; uint32_t num_files = 0; - int bytes_read = 0; - sscanf(data.data(), "%" PRId64 "%n", ×tamp_, &bytes_read); - data.remove_prefix(bytes_read + 1); // +1 for '\n' - sscanf(data.data(), "%" PRIu64 "%n", &sequence_number_, &bytes_read); - data.remove_prefix(bytes_read + 1); // +1 for '\n' - sscanf(data.data(), "%u%n", &num_files, &bytes_read); - data.remove_prefix(bytes_read + 1); // +1 for '\n' + char *next; + timestamp_ = strtoull(data.data(), &next, 10); + data.remove_prefix(next - data.data() + 1); // +1 for '\n' + sequence_number_ = strtoull(data.data(), &next, 10); + data.remove_prefix(next - data.data() + 1); // +1 for '\n' + num_files = strtoul(data.data(), &next, 10); + data.remove_prefix(next - data.data() + 1); // +1 for '\n' std::vector files; + Slice checksum_prefix("crc32 "); + for (uint32_t i = 0; s.ok() && i < num_files; ++i) { auto line = GetSliceUntil(&data, '\n'); std::string filename = GetSliceUntil(&line, ' ').ToString(); @@ -1188,9 +1191,9 @@ Status BackupEngineImpl::BackupMeta::LoadFromFile( } uint32_t checksum_value = 0; - if (line.starts_with("crc32 ")) { - line.remove_prefix(6); - sscanf(line.data(), "%u", &checksum_value); + if (line.starts_with(checksum_prefix)) { + line.remove_prefix(checksum_prefix.size()); + checksum_value = strtoul(line.data(), nullptr, 10); if (memcmp(line.data(), std::to_string(checksum_value).c_str(), line.size() - 1) != 0) { return Status::Corruption("Invalid checksum value"); From a8c5564a9de2014306887d8121328caeffb6480d Mon Sep 17 00:00:00 2001 From: Robert Date: Sun, 4 Jan 2015 12:06:59 +0800 Subject: [PATCH 2/3] Do not issue extra GetFileSize() calls when loading BackupMeta. --- utilities/backupable/backupable_db.cc | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/utilities/backupable/backupable_db.cc b/utilities/backupable/backupable_db.cc index ca7521ff3..2e04488cb 100644 --- a/utilities/backupable/backupable_db.cc +++ b/utilities/backupable/backupable_db.cc @@ -185,6 +185,13 @@ class BackupEngineImpl : public BackupEngine { return files_.empty(); } + const FileInfo* GetFile(const std::string& filename) const { + auto it = file_infos_->find(filename); + if (it == file_infos_->end()) + return nullptr; + return &it->second; + } + const std::vector& GetFiles() { return files_; } @@ -1181,9 +1188,14 @@ Status BackupEngineImpl::BackupMeta::LoadFromFile( std::string filename = GetSliceUntil(&line, ' ').ToString(); uint64_t size; - s = env_->GetFileSize(backup_dir + "/" + filename, &size); - if (!s.ok()) { - return s; + const FileInfo* file_info = GetFile(filename); + if (file_info != nullptr) { + size = file_info->size; + } else { + s = env_->GetFileSize(backup_dir + "/" + filename, &size); + if (!s.ok()) { + return s; + } } if (line.empty()) { From 49376bfe87cb4bb8aa7667d1258a5248054d54a2 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 5 Jan 2015 21:20:06 +0800 Subject: [PATCH 3/3] Fix errors when using -Wshorten-64-to-32. --- utilities/backupable/backupable_db.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/utilities/backupable/backupable_db.cc b/utilities/backupable/backupable_db.cc index 2e04488cb..2a526c940 100644 --- a/utilities/backupable/backupable_db.cc +++ b/utilities/backupable/backupable_db.cc @@ -1176,7 +1176,7 @@ Status BackupEngineImpl::BackupMeta::LoadFromFile( data.remove_prefix(next - data.data() + 1); // +1 for '\n' sequence_number_ = strtoull(data.data(), &next, 10); data.remove_prefix(next - data.data() + 1); // +1 for '\n' - num_files = strtoul(data.data(), &next, 10); + num_files = static_cast(strtoul(data.data(), &next, 10)); data.remove_prefix(next - data.data() + 1); // +1 for '\n' std::vector files; @@ -1205,7 +1205,8 @@ Status BackupEngineImpl::BackupMeta::LoadFromFile( uint32_t checksum_value = 0; if (line.starts_with(checksum_prefix)) { line.remove_prefix(checksum_prefix.size()); - checksum_value = strtoul(line.data(), nullptr, 10); + checksum_value = static_cast( + strtoul(line.data(), nullptr, 10)); if (memcmp(line.data(), std::to_string(checksum_value).c_str(), line.size() - 1) != 0) { return Status::Corruption("Invalid checksum value");