diff --git a/tcmalloc/global_stats.cc b/tcmalloc/global_stats.cc index ae03797c6..98b9bc487 100644 --- a/tcmalloc/global_stats.cc +++ b/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++) { diff --git a/tcmalloc/global_stats.h b/tcmalloc/global_stats.h index 68265e497..fb2f1e1c4 100644 --- a/tcmalloc/global_stats.h +++ b/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); diff --git a/tcmalloc/internal/logging.cc b/tcmalloc/internal/logging.cc index 5e53313c4..c6da7f295 100644 --- a/tcmalloc/internal/logging.cc +++ b/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); diff --git a/tcmalloc/internal/logging.h b/tcmalloc/internal/logging.h index c0cd16d04..d979cb8af 100644 --- a/tcmalloc/internal/logging.h +++ b/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) diff --git a/tcmalloc/internal/logging_test.cc b/tcmalloc/internal/logging_test.cc index 8ca8d2e47..0928d856e 100644 --- a/tcmalloc/internal/logging_test.cc +++ b/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 diff --git a/tcmalloc/internal/memory_stats.cc b/tcmalloc/internal/memory_stats.cc index c62ecd176..db594777c 100644 --- a/tcmalloc/internal/memory_stats.cc +++ b/tcmalloc/internal/memory_stats.cc @@ -57,15 +57,15 @@ struct FDCloser { bool GetMemoryStatsFromCallback( MemoryStats& stats, absl::FunctionRef read) { - ssize_t rc = read(stats.buf, MemoryStats::kBufSize); - if (rc < 0 || rc >= static_cast(MemoryStats::kBufSize)) { + char buf[1024]; + ssize_t rc = read(buf, sizeof(buf)); + if (rc < 0 || rc >= static_cast(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 { diff --git a/tcmalloc/internal/memory_stats.h b/tcmalloc/internal/memory_stats.h index 1711efd1d..6f546a0ee 100644 --- a/tcmalloc/internal/memory_stats.h +++ b/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 diff --git a/tcmalloc/testing/BUILD b/tcmalloc/testing/BUILD index edb5a0685..7e34f467f 100644 --- a/tcmalloc/testing/BUILD +++ b/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", diff --git a/tcmalloc/testing/get_stats_test.cc b/tcmalloc/testing/get_stats_test.cc index 3a3af7e49..ac00142e1 100644 --- a/tcmalloc/testing/get_stats_test.cc +++ b/tcmalloc/testing/get_stats_test.cc @@ -29,13 +29,10 @@ #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" @@ -43,12 +40,7 @@ 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;