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
tdfischer · Aug 13, 2014 · 0c9dc9f · 0c9dc9f
1 parent 4808177
commit 0c9dc9f
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 24 deletions.
diff --git 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
@@ -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 <inttypes.h>
 
 #include <ctype.h>
 #include <stdio.h>
@@ -83,11 +84,17 @@ std::string TableFileName(const std::vector<DbPath>& 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
@@ -61,7 +61,11 @@ extern std::string MakeTableFileName(const std::string& name, uint64_t number);
 extern std::string TableFileName(const std::vector<DbPath>& 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
@@ -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
@@ -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,