Skip to content

Commit

Permalink
pw_allocator: Separate PMR from Allocator
Browse files Browse the repository at this point in the history
In CL 207170, a decision was made to include an `Allocator::as_pmr`
method to facilitate discovery and usage of the PMR support. This
meant Allocator depended on a more specific type, AsPmrAllocator,
and deviated from the pattern of all other forwarding allocators
that are constructed by passing an allocator reference.

This CL cleans up the PMR code by removing the `as_pmr` method,
and renaming the type to just `PmrAllocator`. This makes it easier
for downstream projects to include only what they use, and makes
the usage more similar to other allocator types.

Change-Id: I57b8f94f5be26eb191f68dbdefde2230457f1a7f
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/246412
Lint: Lint 🤖 <[email protected]>
Commit-Queue: Aaron Green <[email protected]>
Reviewed-by: Taylor Cramer <[email protected]>
  • Loading branch information
nopsledder authored and CQ Bot Account committed Nov 4, 2024
1 parent 54ca0db commit 1dcac6a
Show file tree
Hide file tree
Showing 20 changed files with 163 additions and 191 deletions.
2 changes: 1 addition & 1 deletion docs/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ _doxygen_input_files = [ # keep-sorted: start
"$dir_pw_alignment/public/pw_alignment/alignment.h",
"$dir_pw_allocator/public/pw_allocator/allocator.h",
"$dir_pw_allocator/public/pw_allocator/allocator_as_pool.h",
"$dir_pw_allocator/public/pw_allocator/as_pmr_allocator.h",
"$dir_pw_allocator/public/pw_allocator/best_fit_block_allocator.h",
"$dir_pw_allocator/public/pw_allocator/block.h",
"$dir_pw_allocator/public/pw_allocator/block_allocator.h",
Expand All @@ -179,6 +178,7 @@ _doxygen_input_files = [ # keep-sorted: start
"$dir_pw_allocator/public/pw_allocator/libc_allocator.h",
"$dir_pw_allocator/public/pw_allocator/metrics.h",
"$dir_pw_allocator/public/pw_allocator/null_allocator.h",
"$dir_pw_allocator/public/pw_allocator/pmr_allocator.h",
"$dir_pw_allocator/public/pw_allocator/pool.h",
"$dir_pw_allocator/public/pw_allocator/size_reporter.h",
"$dir_pw_allocator/public/pw_allocator/synchronized_allocator.h",
Expand Down
37 changes: 23 additions & 14 deletions pw_allocator/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,9 @@ cc_library(
name = "allocator",
srcs = [
"allocator.cc",
"as_pmr_allocator.cc",
],
hdrs = [
"public/pw_allocator/allocator.h",
"public/pw_allocator/as_pmr_allocator.h",
],
includes = ["public"],
deps = [
Expand Down Expand Up @@ -344,6 +342,17 @@ cc_library(
],
)

cc_library(
name = "pmr_allocator",
srcs = ["pmr_allocator.cc"],
hdrs = ["public/pw_allocator/pmr_allocator.h"],
includes = ["public"],
deps = [
":allocator",
":config",
],
)

cc_library(
name = "pool",
hdrs = ["public/pw_allocator/pool.h"],
Expand Down Expand Up @@ -529,18 +538,6 @@ pw_cc_test(
],
)

pw_cc_test(
name = "as_pmr_allocator_test",
srcs = [
"as_pmr_allocator_test.cc",
],
deps = [
":allocator",
":testing",
"//pw_unit_test",
],
)

pw_cc_test(
name = "best_fit_block_allocator_test",
srcs = ["best_fit_block_allocator_test.cc"],
Expand Down Expand Up @@ -722,6 +719,18 @@ pw_cc_test(
],
)

pw_cc_test(
name = "pmr_allocator_test",
srcs = [
"pmr_allocator_test.cc",
],
deps = [
":pmr_allocator",
":testing",
"//pw_unit_test",
],
)

pw_cc_test(
name = "synchronized_allocator_test",
srcs = [
Expand Down
52 changes: 29 additions & 23 deletions pw_allocator/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,14 @@ config("test_config") {

pw_source_set("allocator") {
public_configs = [ ":public_include_path" ]
public = [
"public/pw_allocator/allocator.h",
"public/pw_allocator/as_pmr_allocator.h",
]
public = [ "public/pw_allocator/allocator.h" ]
public_deps = [
":config",
":deallocator",
"$dir_pw_assert:check",
dir_pw_result,
]
sources = [
"allocator.cc",
"as_pmr_allocator.cc",
]
sources = [ "allocator.cc" ]
}

pw_source_set("allocator_as_pool") {
Expand Down Expand Up @@ -299,6 +293,16 @@ pw_source_set("null_allocator") {
sources = [ "null_allocator.cc" ]
}

pw_source_set("pmr_allocator") {
public_configs = [ ":public_include_path" ]
public = [ "public/pw_allocator/pmr_allocator.h" ]
public_deps = [
":allocator",
":config",
]
sources = [ "pmr_allocator.cc" ]
}

pw_source_set("pool") {
public_configs = [ ":public_include_path" ]
public = [ "public/pw_allocator/pool.h" ]
Expand Down Expand Up @@ -440,14 +444,6 @@ pw_test("allocator_test") {
sources = [ "allocator_test.cc" ]
}

pw_test("as_pmr_allocator_test") {
deps = [
":allocator",
":testing",
]
sources = [ "as_pmr_allocator_test.cc" ]
}

pw_test("best_fit_block_allocator_test") {
deps = [
":best_fit_block_allocator",
Expand Down Expand Up @@ -581,6 +577,14 @@ pw_test("null_allocator_test") {
sources = [ "null_allocator_test.cc" ]
}

pw_test("pmr_allocator_test") {
deps = [
":pmr_allocator",
":testing",
]
sources = [ "pmr_allocator_test.cc" ]
}

pw_test("synchronized_allocator_test") {
enable_if =
pw_sync_BINARY_SEMAPHORE_BACKEND != "" && pw_sync_MUTEX_BACKEND != "" &&
Expand Down Expand Up @@ -637,8 +641,9 @@ pw_test("worst_fit_block_allocator_test") {
pw_test_group("tests") {
tests = [
":allocator_as_pool_test",

# ":allocator_nc_test",
":allocator_test",
":as_pmr_allocator_test",
":best_fit_block_allocator_test",
":block_test",
":bucket_allocator_test",
Expand All @@ -655,9 +660,10 @@ pw_test_group("tests") {
":layout_test",
":libc_allocator_test",
":null_allocator_test",
":typed_pool_test",
":pmr_allocator_test",
":synchronized_allocator_test",
":tracking_allocator_test",
":typed_pool_test",
":unique_ptr_test",
":worst_fit_block_allocator_test",
]
Expand Down Expand Up @@ -739,16 +745,16 @@ pw_size_diff("concrete_allocators_size_report") {
pw_size_diff("forwarding_allocators_size_report") {
title = "Sizes of various forwarding allocator implementations"
binaries = [
{
target = "size_report:as_pmr_allocator"
base = "size_report:as_pmr_allocator_base"
label = "AsPmrAllocator"
},
{
target = "size_report:fallback_allocator"
base = "size_report:fallback_allocator_base"
label = "FallbackAllocator"
},
{
target = "size_report:pmr_allocator"
base = "size_report:pmr_allocator_base"
label = "AsPmrAllocator"
},
{
target = "size_report:synchronized_allocator_isl"
base = "size_report:first_fit_block_allocator"
Expand Down
37 changes: 23 additions & 14 deletions pw_allocator/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ pw_add_library(pw_allocator.test_config INTERFACE
pw_add_library(pw_allocator.allocator STATIC
HEADERS
public/pw_allocator/allocator.h
public/pw_allocator/as_pmr_allocator.h
PUBLIC_INCLUDES
public
PUBLIC_DEPS
Expand All @@ -46,7 +45,6 @@ pw_add_library(pw_allocator.allocator STATIC
pw_result
SOURCES
allocator.cc
as_pmr_allocator.cc
)

pw_add_library(pw_allocator.allocator_as_pool STATIC
Expand Down Expand Up @@ -313,6 +311,17 @@ pw_add_library(pw_allocator.null_allocator STATIC
pw_allocator.allocator
)

pw_add_library(pw_allocator.pmr_allocator STATIC
HEADERS
public/pw_allocator/pmr_allocator.h
PUBLIC_INCLUDES
public
PUBLIC_DEPS
pw_allocator.allocator
SOURCES
pmr_allocator.cc
)

pw_add_library(pw_allocator.pool INTERFACE
HEADERS
public/pw_allocator/pool.h
Expand Down Expand Up @@ -475,18 +484,6 @@ pw_add_test(pw_allocator.allocator_test
pw_allocator
)

pw_add_test(pw_allocator.as_pmr_allocator_test
SOURCES
as_pmr_allocator_test.cc
PRIVATE_DEPS
pw_allocator.allocator
pw_allocator.testing
pw_unit_test
GROUPS
modules
pw_allocator
)

pw_add_test(pw_allocator.best_fit_block_allocator_test
SOURCES
best_fit_block_allocator_test.cc
Expand Down Expand Up @@ -671,6 +668,18 @@ pw_add_test(pw_allocator.null_allocator_test
pw_allocator
)

pw_add_test(pw_allocator.pmr_allocator_test
SOURCES
pmr_allocator_test.cc
PRIVATE_DEPS
pw_allocator.pmr_allocator
pw_allocator.testing
pw_unit_test
GROUPS
modules
pw_allocator
)

pw_add_test(pw_allocator.synchronized_allocator_test
SOURCES
synchronized_allocator_test.cc
Expand Down
8 changes: 4 additions & 4 deletions pw_allocator/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,11 @@ AllocatorAsPool
.. doxygenclass:: pw::allocator::AllocatorAsPool
:members:

.. _module-pw_allocator-api-as_pmr_allocator:
.. _module-pw_allocator-api-pmr_allocator:

AsPmrAllocator
==============
.. doxygenclass:: pw::allocator::AsPmrAllocator
PmrAllocator
============
.. doxygenclass:: pw::allocator::PmrAllocator
:members:

.. _module-pw_allocator-api-fallback_allocator:
Expand Down
2 changes: 1 addition & 1 deletion pw_allocator/design.rst
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ incorporating many of its ideas. :ref:`module-pw_allocator-api-allocator` in
particular is similar to `std::pmr::memory_resource`_.

This similarity is most evident in the PMR adapter class,
:ref:`module-pw_allocator-api-as_pmr_allocator`. This adapter allows any
:ref:`module-pw_allocator-api-pmr_allocator`. This adapter allows any
:ref:`module-pw_allocator-api-allocator` to be used as a
`std::pmr::polymorphic_allocator`_ with any standard library that
`can use an allocator`_. Refer to the guides on how to
Expand Down
2 changes: 1 addition & 1 deletion pw_allocator/examples/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ pw_cc_test(
testonly = True,
srcs = ["pmr.cc"],
deps = [
"//pw_allocator:allocator",
"//pw_allocator:pmr_allocator",
"//pw_allocator:testing",
],
)
Expand Down
2 changes: 1 addition & 1 deletion pw_allocator/examples/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pw_test("metrics") {

pw_test("pmr") {
deps = [
"$dir_pw_allocator:allocator",
"$dir_pw_allocator:pmr_allocator",
"$dir_pw_allocator:testing",
]
sources = [ "pmr.cc" ]
Expand Down
4 changes: 2 additions & 2 deletions pw_allocator/examples/pmr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#include <string_view>

#include "pw_allocator/allocator.h"
#include "pw_allocator/as_pmr_allocator.h"
#include "pw_allocator/pmr_allocator.h"

namespace examples {

Expand All @@ -39,7 +39,7 @@ class LibraryIndex {
}

private:
pw::allocator::AsPmrAllocator allocator_;
pw::allocator::PmrAllocator allocator_;
MapType by_author_;
};

Expand Down
6 changes: 3 additions & 3 deletions pw_allocator/guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ While there are
:ref:`module-pw_allocator-design-differences-with-polymorphic-allocators`, an
:ref:`module-pw_allocator-api-allocator` can be used with these containers by
wrapping them with a PMR adapter type,
:ref:`module-pw_allocator-api-as_pmr_allocator`:
:ref:`module-pw_allocator-api-pmr_allocator`:

.. literalinclude:: examples/pmr.cc
:language: cpp
Expand All @@ -211,7 +211,7 @@ wrapping them with a PMR adapter type,
.. Warning::
The standard library containers expect their allocators to throw an exception
on allocation failure, and do not check for failure themselves. If
exceptions are disabled, :ref:`module-pw_allocator-api-as_pmr_allocator`
exceptions are disabled, :ref:`module-pw_allocator-api-pmr_allocator`
instead **asserts** that allocation succeeded. Care must be taken in this
case to ensure that memory is not exhausted.

Expand Down Expand Up @@ -282,7 +282,7 @@ Consult the :ref:`module-pw_allocator-api` for additional details.

- :ref:`module-pw_allocator-api-fallback_allocator`: Dispatches first to a
primary allocator, and, if that fails, to a secondary allocator.
- :ref:`module-pw_allocator-api-as_pmr_allocator`: Adapts an allocator to be a
- :ref:`module-pw_allocator-api-pmr_allocator`: Adapts an allocator to be a
``std::pmr::polymorphic_allocator``, which can be used with standard library
containers that `use allocators`_, such as ``std::pmr::vector<T>``.
- :ref:`module-pw_allocator-api-synchronized_allocator`: Synchronizes access to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@
// License for the specific language governing permissions and limitations under
// the License.

#include "pw_allocator/as_pmr_allocator.h"
#include "pw_allocator/pmr_allocator.h"

#include "pw_allocator/allocator.h"
#include "pw_assert/check.h"

namespace pw::allocator {
namespace internal {
namespace pw::allocator::internal {

void* MemoryResource::do_allocate(size_t bytes, size_t alignment) {
void* ptr = nullptr;
Expand Down Expand Up @@ -58,5 +56,4 @@ bool MemoryResource::do_is_equal(
return false;
}

} // namespace internal
} // namespace pw::allocator
} // namespace pw::allocator::internal
Loading

0 comments on commit 1dcac6a

Please sign in to comment.