Return pageflags bitmap(s) by value instead of output parameter.

tcmalloc/huge_page_filler.h

@@ -2721,14 +2721,10 @@ class HugePageUnbackedTrackerTreatment final : public HugePageTreatment {
             bitmaps.unbacked, pages_per_huge_page, ReductionOp::kAll);
         state.swapped = Scale<kPagesPerHugePage.raw_num()>(
             bitmaps.swapped, pages_per_huge_page, ReductionOp::kAny);
-        if (pf) {
-          // TODO(b/525422238): Return by value rather than use an output
-          // parameter.
-          ResidencyBitmap b;
-          pf->GetSinglePageBitmaps(tracker->location().start_addr(), b);
-          state.stale = Scale<kPagesPerHugePage.raw_num()>(
-              b, pages_per_huge_page, ReductionOp::kAny);
-        }
+        auto single_page_bitmaps =
+            pf->GetSinglePageBitmaps(tracker->location().start_addr());
+        state.stale = Scale<kPagesPerHugePage.raw_num()>(
+            single_page_bitmaps.stale, pages_per_huge_page, ReductionOp::kAny);
 
         const bool backoff =
             treatment_stats_.collapse_time_max_cycles > max_collapse_cycles;

tcmalloc/huge_page_filler_fuzz.cc

@@ -124,9 +124,8 @@ class FakePageFlags : public PageFlagsBase {
     return PageStats{};
   }
 
-  absl::StatusCode GetSinglePageBitmaps(const void* addr,
-                                        ResidencyBitmap& stale) override {
-    return absl::StatusCode::kUnimplemented;
+  PageFlagsBitmaps GetSinglePageBitmaps(const void* addr) override {
+    return {.status = absl::StatusCode::kUnimplemented};
   }
 
   std::optional<bool> IsHugepageBacked(const void* addr) override {

tcmalloc/huge_page_filler_test.cc

@@ -289,10 +289,11 @@ class FakePageFlags : public PageFlagsBase {
     return PageStats{};
   }
 
-  absl::StatusCode GetSinglePageBitmaps(const void* addr,
-                                        ResidencyBitmap& stale) override {
-    stale.SetBit(0);
-    return absl::StatusCode::kOk;
+  PageFlagsBitmaps GetSinglePageBitmaps(const void* addr) override {
+    PageFlagsBitmaps ret;
+    ret.stale.SetBit(0);
+    ret.status = absl::StatusCode::kOk;
+    return ret;
   }
 
   void MarkHugePageBacked(void* addr, bool is_hugepage_backed) {

tcmalloc/internal/BUILD

@@ -1014,6 +1014,7 @@ cc_test(
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/container:flat_hash_set",
         "@com_google_absl//absl/log:check",
+        "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/time",

tcmalloc/internal/pageflags.cc

@@ -339,21 +339,25 @@ std::optional<PageStats> PageFlags::Get(const void* const addr,
   }
   return ret;
 }
-absl::StatusCode PageFlags::GetSinglePageBitmaps(const void* addr,
-                                                 ResidencyBitmap& stale) {
+PageFlagsBase::PageFlagsBitmaps PageFlags::GetSinglePageBitmaps(
+    const void* addr) {
+  PageFlagsBitmaps ret;
   uintptr_t currPage = reinterpret_cast<uintptr_t>(addr);
   if ((currPage & (kHugePageSize - 1)) != 0) {
     TC_LOG("Address is not hugepage aligned");
-    return absl::StatusCode::kFailedPrecondition;
+    ret.status = absl::StatusCode::kFailedPrecondition;
+    return ret;
   }
 
   if (fd_ < 0) {
-    return absl::StatusCode::kUnavailable;
+    ret.status = absl::StatusCode::kUnavailable;
+    return ret;
   }
 
   auto res = Seek(currPage);
   if (res != absl::StatusCode::kOk) {
-    return res;
+    ret.status = res;
+    return ret;
   }
 
   const size_t kHardwarePagesInHugePage = kHugePageSize / GetPageSize();
@@ -365,7 +369,8 @@ absl::StatusCode PageFlags::GetSinglePageBitmaps(const void* addr,
                                  kSizeOfHugepageInPagemap, nullptr);
   if (status != kSizeOfHugepageInPagemap) {
     TC_LOG("Could not read from pageflags file");
-    return absl::StatusCode::kUnavailable;
+    ret.status = absl::StatusCode::kUnavailable;
+    return ret;
   }
 
   last_head_read_ = -1;
@@ -379,7 +384,8 @@ absl::StatusCode PageFlags::GetSinglePageBitmaps(const void* addr,
     }
     if (PageTail(flags)) {
       if (ABSL_PREDICT_FALSE(last_head_read_ == -1)) {
-        return absl::StatusCode::kFailedPrecondition;
+        ret.status = absl::StatusCode::kFailedPrecondition;
+        return ret;
       }
       flags = last_head_read_;
     }
@@ -390,16 +396,17 @@ absl::StatusCode PageFlags::GetSinglePageBitmaps(const void* addr,
       }
     } else {
       if (stale_start != -1) {
-        stale.SetRange(stale_start, i - stale_start);
+        ret.stale.SetRange(stale_start, i - stale_start);
         stale_start = -1;
       }
     }
   }
   if (stale_start != -1) {
-    stale.SetRange(stale_start, kHardwarePagesInHugePage - stale_start);
+    ret.stale.SetRange(stale_start, kHardwarePagesInHugePage - stale_start);
   }
 
-  return absl::StatusCode::kOk;
+  ret.status = absl::StatusCode::kOk;
+  return ret;
 }
 
 uint64_t PageFlags::MaybeReadStaleScanSeconds(const char* filename) {

tcmalloc/internal/pageflags.h

@@ -68,8 +68,13 @@ class PageFlagsBase {
   PageFlagsBase& operator=(PageFlagsBase&&) = delete;
   virtual std::optional<bool> IsHugepageBacked(const void* addr) = 0;
   virtual std::optional<PageStats> Get(const void* addr, size_t size) = 0;
-  virtual absl::StatusCode GetSinglePageBitmaps(const void* addr,
-                                                ResidencyBitmap& stale) = 0;
+
+  struct PageFlagsBitmaps {
+    ResidencyBitmap stale;
+    absl::StatusCode status;
+  };
+
+  virtual PageFlagsBitmaps GetSinglePageBitmaps(const void* addr) = 0;
 };
 
 // PageFlags offers a look at kernel page flags to identify pieces of memory as
@@ -101,8 +106,7 @@ class PageFlags final : public PageFlagsBase {
   // dynamically allocate memory when needed.  Using std::optional allows us to
   // use the function in places where memory allocation is prohibited.
   std::optional<PageStats> Get(const void* addr, size_t size) override;
-  absl::StatusCode GetSinglePageBitmaps(const void* addr,
-                                        ResidencyBitmap& stale) override;
+  PageFlagsBitmaps GetSinglePageBitmaps(const void* addr) override;
   std::optional<bool> IsHugepageBacked(const void* addr) override;
 
  private:

tcmalloc/internal/pageflags_test.cc

@@ -81,9 +81,8 @@ class PageFlagsFriend {
     return r_.IsHugepageBacked(addr);
   }
 
-  decltype(auto) GetSinglePageBitmaps(const void* addr,
-                                      ResidencyBitmap& stale) {
-    return r_.GetSinglePageBitmaps(addr, stale);
+  decltype(auto) GetSinglePageBitmaps(const void* addr) {
+    return r_.GetSinglePageBitmaps(addr);
   }
 
   void SetCachedScanSeconds(
@@ -608,23 +607,20 @@ TEST(StaleSeconds, TextOverflow) {
 TEST(PageFlagsTest, GetSinglePageBitmapsErrorCases) {
   {
     PageFlagsFriend s;
-    ResidencyBitmap stale;
-    EXPECT_EQ(s.GetSinglePageBitmaps(reinterpret_cast<const void*>(1), stale),
+    EXPECT_EQ(s.GetSinglePageBitmaps(reinterpret_cast<const void*>(1)).status,
               absl::StatusCode::kFailedPrecondition);
   }
   {
     PageFlagsFriend s("/dev/null/impossible");
-    ResidencyBitmap stale;
-    EXPECT_EQ(s.GetSinglePageBitmaps(nullptr, stale),
+    EXPECT_EQ(s.GetSinglePageBitmaps(nullptr).status,
               absl::StatusCode::kUnavailable);
   }
   {
     std::string fake_pageflags =
         absl::StrCat(testing::TempDir(), "/fake_pageflags_short");
     SetContents(fake_pageflags, "x");
     PageFlagsFriend s(fake_pageflags);
-    ResidencyBitmap stale;
-    EXPECT_EQ(s.GetSinglePageBitmaps(nullptr, stale),
+    EXPECT_EQ(s.GetSinglePageBitmaps(nullptr).status,
               absl::StatusCode::kUnavailable);
   }
   {
@@ -639,8 +635,7 @@ TEST(PageFlagsTest, GetSinglePageBitmapsErrorCases) {
     SetContents(fake_pageflags, content);
 
     PageFlagsFriend s(fake_pageflags);
-    ResidencyBitmap stale;
-    EXPECT_EQ(s.GetSinglePageBitmaps(nullptr, stale),
+    EXPECT_EQ(s.GetSinglePageBitmaps(nullptr).status,
               absl::StatusCode::kFailedPrecondition);
   }
 }
@@ -661,11 +656,10 @@ TEST(PageFlagsTest, GetSinglePageBitmapsSuccess) {
   SetContents(fake_pageflags, content);
 
   PageFlagsFriend s(fake_pageflags);
-  ResidencyBitmap stale;
-  stale.Clear();
 
-  EXPECT_EQ(s.GetSinglePageBitmaps(nullptr, stale), absl::StatusCode::kOk);
-  EXPECT_EQ(stale.CountBits(), kMaxResidencyBits / 2);
+  auto ret = s.GetSinglePageBitmaps(nullptr);
+  EXPECT_EQ(ret.status, absl::StatusCode::kOk);
+  EXPECT_EQ(ret.stale.CountBits(), kMaxResidencyBits / 2);
 }
 
 }  // namespace

tcmalloc/internal/profile_builder_test.cc

@@ -40,6 +40,7 @@
 #include "absl/container/flat_hash_map.h"
 #include "absl/container/flat_hash_set.h"
 #include "absl/log/check.h"
+#include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
@@ -71,9 +72,8 @@ class StubPageFlags final : public PageFlagsBase {
  public:
   StubPageFlags() = default;
   ~StubPageFlags() override = default;
-  absl::StatusCode GetSinglePageBitmaps(const void* addr,
-                                        ResidencyBitmap& stale) override {
-    return absl::StatusCode::kUnimplemented;
+  PageFlagsBitmaps GetSinglePageBitmaps(const void* addr) override {
+    return {.status = absl::StatusCode::kUnimplemented};
   }
   std::optional<PageStats> Get(const void* addr, size_t size) override {
     PageStats ret;