Revert: Record contents of /proc/self/statm when the rss obtained from the file is negative or a very large positive number

tcmalloc/global_stats.cc

@@ -39,7 +39,6 @@
 #include "tcmalloc/internal/cpu_utils.h"
 #include "tcmalloc/internal/logging.h"
 #include "tcmalloc/internal/memory_stats.h"
-#include "tcmalloc/internal/memory_tag.h"
 #include "tcmalloc/internal/optimization.h"
 #include "tcmalloc/internal/pageflags.h"
 #include "tcmalloc/internal/percpu.h"
@@ -704,25 +703,6 @@ void DumpStats(Printer& out, int level) {
   }
 }
 
-void PrintMemoryStatsInPbtxt(PbtxtRegion& region) {
-  MemoryStats memstats;
-  if (GetMemoryStats(memstats)) {
-    PrintMemoryStatsInPbtxt(region, memstats);
-  }
-}
-
-void PrintMemoryStatsInPbtxt(PbtxtRegion& region, const MemoryStats& stats) {
-  // We have observed negative and large positive values for total_resident
-  // memory.  We record the data from the /proc/self/statm file to identify
-  // the underlying issue.
-  if (stats.rss < 0 || stats.rss >= (1LL << 60)) {
-    region.PrintString("proc_self_statm",
-                       absl::string_view(stats.buf, stats.rc));
-  }
-  region.PrintI64("total_resident", uint64_t(stats.rss));
-  region.PrintI64("total_mapped", uint64_t(stats.vss));
-}
-
 void DumpStatsInPbtxt(Printer& out, int level) {
   TCMallocStats stats;
   uint64_t class_count[kNumClasses];
@@ -799,7 +779,11 @@ void DumpStatsInPbtxt(Printer& out, int level) {
   }
 
   // Print total process stats (inclusive of non-malloc sources).
-  PrintMemoryStatsInPbtxt(region);
+  MemoryStats memstats;
+  if (GetMemoryStats(memstats)) {
+    region.PrintI64("total_resident", uint64_t(memstats.rss));
+    region.PrintI64("total_mapped", uint64_t(memstats.vss));
+  }
 
   const size_t num_nodes = tc_globals.numa_topology().num_nodes();
   for (size_t node = 0; node < num_nodes; node++) {

tcmalloc/global_stats.h

@@ -22,7 +22,6 @@
 #include "tcmalloc/arena.h"
 #include "tcmalloc/internal/config.h"
 #include "tcmalloc/internal/logging.h"
-#include "tcmalloc/internal/memory_stats.h"
 #include "tcmalloc/metadata_object_allocator.h"
 #include "tcmalloc/page_allocator.h"
 #include "tcmalloc/pages.h"
@@ -80,11 +79,6 @@ size_t SlackBytes(const BackingStats& stats);
 void DumpStats(Printer& out, int level);
 void DumpStatsInPbtxt(Printer& out, int level);
 
-// TODO(b/484431489): remove the functions PrintMemoryStatsInPbtxt from the
-// header file.  These functions had been exposed for testing purposes only.
-void PrintMemoryStatsInPbtxt(PbtxtRegion& region);
-void PrintMemoryStatsInPbtxt(PbtxtRegion& region, const MemoryStats& stats);
-
 bool GetNumericProperty(const char* name_data, size_t name_size,
                         size_t* absl_nonnull value);
 

tcmalloc/internal/logging.cc

@@ -274,12 +274,6 @@ void PbtxtRegion::PrintRaw(absl::string_view key, absl::string_view value) {
   out_->Append(" ", key, ": ", value);
 }
 
-void PbtxtRegion::PrintString(absl::string_view key, absl::string_view value) {
-  // We need to quote-delimit the string to store it correctly:
-  // https://protobuf.dev/reference/protobuf/textformat-spec/#string.
-  out_->Append(" ", key, ": \"", value, "\"");
-}
-
 #ifndef NDEBUG
 void PbtxtRegion::InjectValues(int64_t* i64, double* d, bool* b) {
   injected_i64.store(i64, std::memory_order_release);

tcmalloc/internal/logging.h

@@ -331,9 +331,6 @@ class PbtxtRegion {
   void PrintBool(absl::string_view key, bool value);
   // Useful for enums.
   void PrintRaw(absl::string_view key, absl::string_view value);
-  // TODO(b/484431489): remove the PrintString() function once the bug has been
-  // fixed.
-  void PrintString(absl::string_view key, absl::string_view value);
 
   // Prints 'key subregion'. Return the created subregion.
   PbtxtRegion CreateSubRegion(absl::string_view key)

tcmalloc/internal/logging_test.cc

@@ -253,20 +253,6 @@ TEST(Check, Optional) {
   EXPECT_DEATH(TC_CHECK_EQ(opt1, noopt), "opt1 == noopt \\(1 == \\?\\?\\?\\)");
 }
 
-TEST(PbtxtRegionTest, Print) {
-  char buf[1024] = {};
-  Printer printer(buf, sizeof(buf));
-  PbtxtRegion region(printer, kTop);
-
-  region.PrintI64("foo", 1);
-  region.PrintDouble("bar", 2.0);
-  region.PrintBool("baz", true);
-  region.PrintRaw("xyzzy", "quux");
-  region.PrintString("grault", "garply");
-
-  EXPECT_STREQ(buf, R"( foo: 1 bar: 2 baz: true xyzzy: quux grault: "garply")");
-}
-
 }  // namespace
 }  // namespace tcmalloc_internal
 }  // namespace tcmalloc

tcmalloc/internal/memory_stats.cc

@@ -57,15 +57,15 @@ struct FDCloser {
 bool GetMemoryStatsFromCallback(
     MemoryStats& stats,
     absl::FunctionRef<ssize_t(char* buf, size_t count)> read) {
-  ssize_t rc = read(stats.buf, MemoryStats::kBufSize);
-  if (rc < 0 || rc >= static_cast<ssize_t>(MemoryStats::kBufSize)) {
+  char buf[1024];
+  ssize_t rc = read(buf, sizeof(buf));
+  if (rc < 0 || rc >= static_cast<ssize_t>(sizeof(buf))) {
     return false;
   }
-  stats.rc = rc;
-  stats.buf[rc] = '\0';
+  buf[rc] = '\0';
 
   const size_t pagesize = GetPageSize();
-  absl::string_view contents(stats.buf, rc);
+  absl::string_view contents(buf, rc);
   absl::string_view::size_type start = 0;
   int index = 0;
   do {

tcmalloc/internal/memory_stats.h

@@ -35,9 +35,6 @@ struct MemoryStats {
   int64_t shared;
   int64_t code;
   int64_t data;
-  static constexpr int kBufSize = 1024;
-  char buf[kBufSize];
-  int rc;
 };
 
 // Memory stats of a process

tcmalloc/testing/BUILD

@@ -643,7 +643,6 @@ create_tcmalloc_testsuite(
         "//tcmalloc:malloc_extension",
         "//tcmalloc/internal:config",
         "//tcmalloc/internal:logging",
-        "//tcmalloc/internal:memory_stats",
         "@com_google_absl//absl/base:config",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/time",

tcmalloc/testing/get_stats_test.cc

@@ -29,26 +29,18 @@
 #include "absl/base/config.h"
 #include "absl/strings/match.h"
 #include "absl/strings/str_cat.h"
-#include "absl/strings/string_view.h"
 #include "absl/time/time.h"
 #include "tcmalloc/experiment.h"
 #include "tcmalloc/experiment_config.h"
-#include "tcmalloc/global_stats.h"
 #include "tcmalloc/internal/logging.h"
-#include "tcmalloc/internal/memory_stats.h"
 #include "tcmalloc/malloc_extension.h"
 #include "tcmalloc/parameters.h"
 #include "tcmalloc/testing/testutil.h"
 
 namespace tcmalloc {
 namespace {
 
-using tcmalloc_internal::kTop;
-using tcmalloc_internal::MemoryStats;
 using tcmalloc_internal::Parameters;
-using tcmalloc_internal::PbtxtRegion;
-using tcmalloc_internal::Printer;
-using tcmalloc_internal::PrintMemoryStatsInPbtxt;
 using ::testing::AnyOf;
 using ::testing::ContainsRegex;
 using ::testing::HasSubstr;