Skip to content

Commit

Permalink
Reland "[base/allocator] Support mallinfo() for PartitionAlloc-Everyw…
Browse files Browse the repository at this point in the history
…here."

This reverts commit aa18dd9.

Reason for revert: Not the cause of breakage, as the bot is still
broken. As there was a suspicion this wasn't the culprit to begin
with, relanding.

Original change's description:
> Revert "[base/allocator] Support mallinfo() for PartitionAlloc-Everywhere."
>
> This reverts commit 98e662e.
>
> Reason for revert: this seem to break Win7 builder
> https://ci.chromium.org/p/chromium/builders/ci/Win7%20Tests%20%28dbg%29%281%29/85993
>
>
>
> Original change's description:
> > [base/allocator] Support mallinfo() for PartitionAlloc-Everywhere.
> >
> > mallinfo() is used by MallocDumpProvider, and ProcessMemoryMetrics. The
> > first one can also use the extended API of PartitionAlloc to report
> > data, but the second one uses mallinfo(). To support both, implement
> > mallinfo().
> >
> > Bug: 998048
> > Change-Id: I9d71cd3a7bbf5c766cfc3771907c2f51ac17cace
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2426708
> > Reviewed-by: Will Harris <wfh@chromium.org>
> > Commit-Queue: Benoit L <lizeb@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#819010}
>
> TBR=lizeb@chromium.org,wfh@chromium.org
>
> Change-Id: Ibfbc278d7057808ce1a66150e2a8f516bdaab230
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 998048
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489626
> Reviewed-by: Anatoliy Potapchuk <apotapchuk@chromium.org>
> Commit-Queue: Anatoliy Potapchuk <apotapchuk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#819368}

TBR=lizeb@chromium.org,wfh@chromium.org,apotapchuk@chromium.org

# Not skipping CQ checks because this is a reland.

Bug: 998048
Change-Id: I8b558903c6a1b94c3db472d200472a03c97b8322
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489909
Reviewed-by: Benoit L <lizeb@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819782}
  • Loading branch information
Benoit L authored and Commit Bot committed Oct 22, 2020
1 parent b8ab8da commit 445f750
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 3 deletions.
4 changes: 4 additions & 0 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3382,6 +3382,10 @@ test("base_unittests") {
if (is_win) {
sources += [ "allocator/winheap_stubs_win_unittest.cc" ]
}

if (use_allocator == "partition") {
sources += [ "allocator/allocator_shim_default_dispatch_to_partition_alloc_unittest.cc" ]
}
}

if (enable_base_tracing) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
#include "base/no_destructor.h"
#include "build/build_config.h"

#if defined(OS_POSIX)
#include <malloc.h>
#endif

namespace {

// We would usually make g_root a static local variable, as these are guaranteed
Expand Down Expand Up @@ -188,6 +192,26 @@ size_t PartitionGetSizeEstimate(const AllocatorDispatch*,
return base::ThreadSafePartitionRoot::GetUsableSize(address);
}

class PartitionStatsDumperImpl : public base::PartitionStatsDumper {
public:
PartitionStatsDumperImpl() = default;

void PartitionDumpTotals(
const char* partition_name,
const base::PartitionMemoryStats* memory_stats) override {
stats_ = *memory_stats;
}

void PartitionsDumpBucketStats(
const char* partition_name,
const base::PartitionBucketMemoryStats*) override {}

const base::PartitionMemoryStats& stats() const { return stats_; }

private:
base::PartitionMemoryStats stats_;
};

} // namespace

namespace base {
Expand Down Expand Up @@ -239,10 +263,30 @@ SHIM_ALWAYS_EXPORT int mallopt(int cmd, int value) __THROW {

#endif // !defined(OS_APPLE)

#ifdef HAVE_STRUCT_MALLINFO
#if defined(OS_POSIX)
SHIM_ALWAYS_EXPORT struct mallinfo mallinfo(void) __THROW {
return {};
PartitionStatsDumperImpl allocator_dumper;
Allocator().DumpStats("malloc", true, &allocator_dumper);

PartitionStatsDumperImpl aligned_allocator_dumper;
AlignedAllocator()->DumpStats("posix_memalign", true,
&aligned_allocator_dumper);

struct mallinfo info = {0};
info.arena = 0; // Memory *not* allocated with mmap().

// Memory allocated with mmap(), aka virtual size.
info.hblks = allocator_dumper.stats().total_mmapped_bytes +
aligned_allocator_dumper.stats().total_mmapped_bytes;
// Resident bytes.
info.hblkhd = allocator_dumper.stats().total_resident_bytes +
aligned_allocator_dumper.stats().total_resident_bytes;
// Allocated bytes.
info.uordblks = allocator_dumper.stats().total_active_bytes +
aligned_allocator_dumper.stats().total_active_bytes;

return info;
}
#endif
#endif // defined(OS_POSIX)

} // extern "C"
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/allocator/buildflags.h"
#include "base/compiler_specific.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"

#if defined(OS_POSIX)
#include <malloc.h>
#include <stdlib.h>
#endif

#if BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && \
!defined(MEMORY_TOOL_REPLACES_ALLOCATOR)

// Platforms on which we override weak libc symbols.
#if defined(OS_LINUX) || defined(OS_CHROMEOS)

NOINLINE void FreeForTest(void* data) {
free(data);
}

TEST(PartitionAllocAsMalloc, Mallinfo) {
constexpr int kLargeAllocSize = 10 * 1024 * 1024;
struct mallinfo before = mallinfo();
void* data = malloc(1000);
ASSERT_TRUE(data);
void* aligned_data;
ASSERT_EQ(0, posix_memalign(&aligned_data, 1024, 1000));
ASSERT_TRUE(aligned_data);
void* direct_mapped_data = malloc(kLargeAllocSize);
ASSERT_TRUE(direct_mapped_data);
struct mallinfo after_alloc = mallinfo();

// Something is reported.
EXPECT_GT(after_alloc.hblks, 0);
EXPECT_GT(after_alloc.hblkhd, 0);
EXPECT_GT(after_alloc.uordblks, 0);

EXPECT_GT(after_alloc.hblks, kLargeAllocSize);

// malloc() can reuse memory, so sizes are not necessarily changing, which
// would mean that we need EXPECT_G*E*() rather than EXPECT_GT().
//
// However since we allocate direct-mapped memory, this increases the total.
EXPECT_GT(after_alloc.hblks, before.hblks);
EXPECT_GT(after_alloc.hblkhd, before.hblkhd);
EXPECT_GT(after_alloc.uordblks, before.uordblks);

// a simple malloc() / free() pair can be discarded by the compiler (and is),
// making the test fail. It is sufficient to make |FreeForTest()| a NOINLINE
// function for the call to not be eliminated, but this is required.
FreeForTest(data);
FreeForTest(aligned_data);
FreeForTest(direct_mapped_data);
struct mallinfo after_free = mallinfo();

EXPECT_LT(after_free.hblks, after_alloc.hblks);
EXPECT_LT(after_free.hblkhd, after_alloc.hblkhd);
EXPECT_LT(after_free.uordblks, after_alloc.uordblks);
}

#endif // defined(OS_LINUX) || defined(OS_CHROMEOS)

#endif // BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) && \
// !defined(MEMORY_TOOL_REPLACES_ALLOCATOR)

0 comments on commit 445f750

Please sign in to comment.