From 0c9dc9f8e0bb4c0755830adc3102820a5307102c Mon Sep 17 00:00:00 2001 From: Stanislau Hlebik Date: Wed, 13 Aug 2014 11:57:40 -0700 Subject: [PATCH] Remove malloc from FormatFileNumber Summary: Replace unnecessary malloc with stack allocation Test Plan: make all check Reviewers: sdong Reviewed By: sdong Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D21771 --- db/compaction_picker.cc | 39 ++++++++++++++++++++++++--------------- db/filename.cc | 15 +++++++++++---- db/filename.h | 6 +++++- db/repair.cc | 7 ++++--- db/version_set.cc | 2 +- 5 files changed, 45 insertions(+), 24 deletions(-) diff --git a/db/compaction_picker.cc b/db/compaction_picker.cc index 170a48efe..512abf8fa 100644 --- a/db/compaction_picker.cc +++ b/db/compaction_picker.cc @@ -720,10 +720,11 @@ Compaction* UniversalCompactionPicker::PickCompactionUniversalReadAmp( // first candidate to be compacted. uint64_t candidate_size = f != nullptr? f->compensated_file_size : 0; if (f != nullptr) { - LogToBuffer( - log_buffer, "[%s] Universal: Possible candidate file %s[%d].", - version->cfd_->GetName().c_str(), - FormatFileNumber(f->fd.GetNumber(), f->fd.GetPathId()).c_str(), loop); + char file_num_buf[kFormatFileNumberBufSize]; + FormatFileNumber(f->fd.GetNumber(), f->fd.GetPathId(), file_num_buf, + sizeof(file_num_buf)); + LogToBuffer(log_buffer, "[%s] Universal: Possible candidate file %s[%d].", + version->cfd_->GetName().c_str(), file_num_buf, loop); } // Check if the suceeding files need compaction. @@ -815,12 +816,14 @@ Compaction* UniversalCompactionPicker::PickCompactionUniversalReadAmp( for (unsigned int i = start_index; i < first_index_after; i++) { FileMetaData* f = c->input_version_->files_[level][i]; c->inputs_[0].files.push_back(f); + char file_num_buf[kFormatFileNumberBufSize]; + FormatFileNumber(f->fd.GetNumber(), f->fd.GetPathId(), file_num_buf, + sizeof(file_num_buf)); LogToBuffer(log_buffer, "[%s] Universal: Picking file %s[%d] " "with size %" PRIu64 " (compensated size %" PRIu64 ")\n", - version->cfd_->GetName().c_str(), - FormatFileNumber(f->fd.GetNumber(), f->fd.GetPathId()).c_str(), - i, f->fd.GetFileSize(), f->compensated_file_size); + version->cfd_->GetName().c_str(), file_num_buf, i, + f->fd.GetFileSize(), f->compensated_file_size); } return c; } @@ -854,29 +857,35 @@ Compaction* UniversalCompactionPicker::PickCompactionUniversalSizeAmp( start_index = loop; // Consider this as the first candidate. break; } + char file_num_buf[kFormatFileNumberBufSize]; + FormatFileNumber(f->fd.GetNumber(), f->fd.GetPathId(), file_num_buf, + sizeof(file_num_buf)); LogToBuffer(log_buffer, "[%s] Universal: skipping file %s[%d] compacted %s", - version->cfd_->GetName().c_str(), - FormatFileNumber(f->fd.GetNumber(), f->fd.GetPathId()).c_str(), - loop, " cannot be a candidate to reduce size amp.\n"); + version->cfd_->GetName().c_str(), file_num_buf, loop, + " cannot be a candidate to reduce size amp.\n"); f = nullptr; } if (f == nullptr) { return nullptr; // no candidate files } + char file_num_buf[kFormatFileNumberBufSize]; + FormatFileNumber(f->fd.GetNumber(), f->fd.GetPathId(), file_num_buf, + sizeof(file_num_buf)); LogToBuffer(log_buffer, "[%s] Universal: First candidate file %s[%d] %s", - version->cfd_->GetName().c_str(), - FormatFileNumber(f->fd.GetNumber(), f->fd.GetPathId()).c_str(), - start_index, " to reduce size amp.\n"); + version->cfd_->GetName().c_str(), file_num_buf, start_index, + " to reduce size amp.\n"); // keep adding up all the remaining files for (unsigned int loop = start_index; loop < files.size() - 1; loop++) { f = files[loop]; if (f->being_compacted) { + char file_num_buf[kFormatFileNumberBufSize]; + FormatFileNumber(f->fd.GetNumber(), f->fd.GetPathId(), file_num_buf, + sizeof(file_num_buf)); LogToBuffer( log_buffer, "[%s] Universal: Possible candidate file %s[%d] %s.", - version->cfd_->GetName().c_str(), - FormatFileNumber(f->fd.GetNumber(), f->fd.GetPathId()).c_str(), loop, + version->cfd_->GetName().c_str(), file_num_buf, loop, " is already being compacted. No size amp reduction possible.\n"); return nullptr; } diff --git a/db/filename.cc b/db/filename.cc index 6693423a7..391590f50 100644 --- a/db/filename.cc +++ b/db/filename.cc @@ -6,8 +6,9 @@ // Copyright (c) 2011 The LevelDB Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. See the AUTHORS file for names of contributors. - +#define __STDC_FORMAT_MACROS #include "db/filename.h" +#include #include #include @@ -83,11 +84,17 @@ std::string TableFileName(const std::vector& db_paths, uint64_t number, return MakeTableFileName(path, number); } -std::string FormatFileNumber(uint64_t number, uint32_t path_id) { +const size_t kFormatFileNumberBufSize = 38; + +void FormatFileNumber(uint64_t number, uint32_t path_id, char* out_buf, + size_t out_buf_size) { if (path_id == 0) { - return std::to_string(number); + snprintf(out_buf, out_buf_size, "%" PRIu64, number); } else { - return std::to_string(number) + "(path " + std::to_string(path_id) + ")"; + snprintf(out_buf, out_buf_size, "%" PRIu64 + "(path " + "%" PRIu32 ")", + number, path_id); } } diff --git a/db/filename.h b/db/filename.h index b4992e5f5..17adc94ae 100644 --- a/db/filename.h +++ b/db/filename.h @@ -61,7 +61,11 @@ extern std::string MakeTableFileName(const std::string& name, uint64_t number); extern std::string TableFileName(const std::vector& db_paths, uint64_t number, uint32_t path_id); -extern std::string FormatFileNumber(uint64_t number, uint32_t path_id); +// Sufficient buffer size for FormatFileNumber. +extern const size_t kFormatFileNumberBufSize; + +extern void FormatFileNumber(uint64_t number, uint32_t path_id, char* out_buf, + size_t out_buf_size); // Return the name of the descriptor file for the db named by // "dbname" and the specified incarnation number. The result will be diff --git a/db/repair.cc b/db/repair.cc index b927a4ccb..66ca946a6 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -265,9 +265,10 @@ class Repairer { if (!status.ok()) { std::string fname = TableFileName( options_.db_paths, t.meta.fd.GetNumber(), t.meta.fd.GetPathId()); - Log(options_.info_log, "Table #%s: ignoring %s", - FormatFileNumber(t.meta.fd.GetNumber(), t.meta.fd.GetPathId()) - .c_str(), + char file_num_buf[kFormatFileNumberBufSize]; + FormatFileNumber(t.meta.fd.GetNumber(), t.meta.fd.GetPathId(), + file_num_buf, sizeof(file_num_buf)); + Log(options_.info_log, "Table #%s: ignoring %s", file_num_buf, status.ToString().c_str()); ArchiveFile(fname); } else { diff --git a/db/version_set.cc b/db/version_set.cc index 7c75db0d0..5e61de6c9 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -1393,7 +1393,7 @@ const char* Version::LevelFileSummary(FileSummaryStorage* scratch, for (const auto& f : files_[level]) { int sz = sizeof(scratch->buffer) - len; char sztxt[16]; - AppendHumanBytes(f->fd.GetFileSize(), sztxt, 16); + AppendHumanBytes(f->fd.GetFileSize(), sztxt, sizeof(sztxt)); int ret = snprintf(scratch->buffer + len, sz, "#%" PRIu64 "(seq=%" PRIu64 ",sz=%s,%d) ", f->fd.GetNumber(), f->smallest_seqno, sztxt,