[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone abandoned this revision.
Quuxplusone marked 3 inline comments as done.
Quuxplusone added a comment.

Sure; not that D89057  is making any //more// 
progress... :)


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

Can we please close this review as it's been superseded by D89057 
?


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone marked 11 inline comments as done.
Quuxplusone added inline comments.



Comment at: include/experimental/memory_resource:427
+static _LIBCPP_CONSTEXPR const size_t __default_buffer_capacity = 1024;
+static _LIBCPP_CONSTEXPR const size_t __default_buffer_alignment = 16;
+

strager wrote:
> Nit: Why isn't `__default_buffer_alignment` set to `alignof(max_align_t)` 
> (which I think is 16)? I think `alignof(max_align_t)` is a good, portable 
> default.
Will users be more upset if `__default_buffer_alignment` is 16-instead-of-`???` 
on exotic platform `???`; or more upset if `__default_buffer_alignment` is 
//usually// 16 but then sometimes is `???`-instead-of-16 on exotic platforms? I 
tend to think that "usually 16, sometimes something else" is worse behavior 
software-engineering-wise than "always 16, even when something else might 
arguably be better on exotic platform `???`."



Comment at: include/experimental/memory_resource:431-432
+__chunk_header *__next_;
+char *__start_;
+char *__cur_;
+size_t __align_;

strager wrote:
> Nit: Would it make sense to use `std::byte` instead of `char`?
IMHO "strong no"; `char` is the standard core-language type that takes up one 
byte. `std::byte` is an enum type that we'd have to drag in from a header 
(cost), and otherwise behaves just like `char` (no benefit). But it's certainly 
a non-functional nit and I would not put up a fight if libc++ wanted it that 
way. :)



Comment at: include/experimental/memory_resource:433
+char *__cur_;
+size_t __align_;
+size_t __allocation_size() {

strager wrote:
> > Eric suggests replacing size_t __align_ with uint8_t __log2_align_. I'm 
> > amenable to this idea, but I'd want to know what's the best way to compute 
> > the log2 of a user-supplied number.
> 
> Perhaps `std::log2p1` could help: 
> https://en.cppreference.com/w/cpp/numeric/log2p1
Yes, but that's C++2a; I imagine some fiddliness would be needed to use it in 
C++17 library code (which in turn is actually compiled as C++11 or '14, I 
forget).



Comment at: 
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp:32
+assert(globalMemCounter.checkOutstandingNewEq(0));
+}
+

strager wrote:
> Nit: This test is named allocate_in_geometric_progression. I don't see any 
> multiplication or anything happening in this test case. Did you mean to call 
> this test something else, like release_deletes_upstream_memory?
Prior to the resolution of [LWG 
3120](https://cplusplus.github.io/LWG/issue3120), this test had tested 
something else. I guess now it //should// be more like 
"release_resets_initial_buffer.pass.cpp".



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp:28
+std::experimental::pmr::monotonic_buffer_resource 
mono1(std::experimental::pmr::new_delete_resource());
+std::experimental::pmr::memory_resource & r1 = mono1;
+

strager wrote:
> Nit: What is the purpose of the `r1` variable? Why not use `mono1` everywhere 
> instead (and rename `mono1` to `r1` if you like `r1` better than `mono1` as a 
> name)?
`r1` has a different static type. For example, here it's testing that 
`r1.allocate(50)` actually makes a virtual call to 
`monotonic_buffer_resource::do_allocate`. But I guess it doesn't really matter; 
I could use `mono1` for everything just as well.



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp:48-49
+void *res = r1.allocate(64, 16);
+assert(res == buffer);
+assert(globalMemCounter.checkNewCalledEq(0));
+

strager wrote:
> Nit: These assertions look unrelated to exception safety to me. (The test is 
> named allocate_exception_safety.) I'd delete these assertions.
Depends on your test-overfitting philosophy. These assertions are still 
expected to pass 100% of the time — if they fail, then something is definitely 
wrong. And they're cheap to execute. So IMHO they might as well be there, as 
"self-documenting code."



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp:67-68
+
+upstream.which = std::experimental::pmr::new_delete_resource();
+res = r1.allocate(last_new_size, 16);
+assert(res != buffer);

strager wrote:
> Question about the intended behavior of `monotonic_buffer_resource`:
> 
> If line 68 instead allocated 1 byte, and the byte could fit in the original 
> heap allocation (line 51), should `monotonic_buffer_resource` return a byte 
> from that original resource? In other words, should the following 

[PATCH] D47111: : Implement monotonic_buffer_resource.

[strager via Phabricator via cfe-commits]

strager added inline comments.



Comment at: include/experimental/memory_resource:433
+char *__cur_;
+size_t __align_;
+size_t __allocation_size() {

> Eric suggests replacing size_t __align_ with uint8_t __log2_align_. I'm 
> amenable to this idea, but I'd want to know what's the best way to compute 
> the log2 of a user-supplied number.

Perhaps `std::log2p1` could help: 
https://en.cppreference.com/w/cpp/numeric/log2p1


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[strager via Phabricator via cfe-commits]

strager added inline comments.



Comment at: include/experimental/memory_resource:427
+static _LIBCPP_CONSTEXPR const size_t __default_buffer_capacity = 1024;
+static _LIBCPP_CONSTEXPR const size_t __default_buffer_alignment = 16;
+

Nit: Why isn't `__default_buffer_alignment` set to `alignof(max_align_t)` 
(which I think is 16)? I think `alignof(max_align_t)` is a good, portable 
default.



Comment at: include/experimental/memory_resource:431-432
+__chunk_header *__next_;
+char *__start_;
+char *__cur_;
+size_t __align_;

Nit: Would it make sense to use `std::byte` instead of `char`?



Comment at: 
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp:32
+assert(globalMemCounter.checkOutstandingNewEq(0));
+}
+

Nit: This test is named allocate_in_geometric_progression. I don't see any 
multiplication or anything happening in this test case. Did you mean to call 
this test something else, like release_deletes_upstream_memory?



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp:28
+std::experimental::pmr::monotonic_buffer_resource 
mono1(std::experimental::pmr::new_delete_resource());
+std::experimental::pmr::memory_resource & r1 = mono1;
+

Nit: What is the purpose of the `r1` variable? Why not use `mono1` everywhere 
instead (and rename `mono1` to `r1` if you like `r1` better than `mono1` as a 
name)?



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp:48-49
+void *res = r1.allocate(64, 16);
+assert(res == buffer);
+assert(globalMemCounter.checkNewCalledEq(0));
+

Nit: These assertions look unrelated to exception safety to me. (The test is 
named allocate_exception_safety.) I'd delete these assertions.



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp:52-54
+assert(res != buffer);
+assert(globalMemCounter.checkNewCalledEq(1));
+assert(globalMemCounter.checkDeleteCalledEq(0));

(Same comment as on lines 48-49 above. I'd delete these assertions.)



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp:67-68
+
+upstream.which = std::experimental::pmr::new_delete_resource();
+res = r1.allocate(last_new_size, 16);
+assert(res != buffer);

Question about the intended behavior of `monotonic_buffer_resource`:

If line 68 instead allocated 1 byte, and the byte could fit in the original 
heap allocation (line 51), should `monotonic_buffer_resource` return a byte 
from that original resource? In other words, should the following test pass?

```
repointable_resource upstream(std::experimental::pmr::new_delete_resource());
std::experimental::pmr::monotonic_buffer_resource mono1();

globalMemCounter.reset();
mono1.allocate(1, 1);
const size_t first_new_size = globalMemCounter.last_new_size;
bool mono1_has_spare_capacity = first_new_size == 1;

upstream.which = std::experimental::pmr::null_memory_resource();
try {
mono1.allocate(first_new_size, 1); // Force allocation from upstream.
assert(false);
} catch (const std::bad_alloc&) {
// we expect this
}

globalMemCounter.reset();
mono1.allocate(1, 1);
if (mono1_has_spare_capacity) {
  assert(globalMemCounter.checkNewCalledEq(0));
} else {
  assert(globalMemCounter.checkNewCalledEq(1));
}
```

Similarly, if `monotonic_buffer_resource` was given a buffer, and that buffer 
still has spare capacity, should that capacity be reused? In other words, 
should the following test pass?

```
int first_allocation_size = 20;
int second_allocation_size = 30;

auto *upstream = std::experimental::pmr::null_memory_resource();
char buffer[64];
assert(sizeof buffer >= first_allocation_size + second_allocation_size);
std::experimental::pmr::monotonic_buffer_resource mono1(buffer, sizeof buffer, 
upstream);

globalMemCounter.reset();
void *first_allocation = mono1.allocate(first_allocation_size, 1);
assert(first_allocation == [0]);

try {
mono1.allocate(sizeof buffer, 1); // Force allocation from upstream.
assert(false);
} catch (const std::bad_alloc&) {
// we expect this
}

void *second_allocation = mono1.allocate(second_allocation_size, 1);
assert(second_allocation == [second_allocation_size]);
```

I think these scenarios would be useful to test, in addition to the scenario in 
your test here.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 219756.
Quuxplusone added a comment.
Herald added a subscriber: dexonsmith.

Rebased on master.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,64 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+void *do_allocate(size_t, size_t)
+{ assert(false); }
+
+void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main(int, char**)
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+
+  return 0;
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,53 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+
+#include "count_new.h"
+
+void test(size_t initial_buffer_size)
+{
+globalMemCounter.reset();
+
+std::experimental::pmr::monotonic_buffer_resource mono1(
+initial_buffer_size,
+

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 191814.
Quuxplusone added a comment.
Herald added a subscriber: jdoerfert.

Rebased on master. Added `_NOEXCEPT` to the `resource()` method (this is OK per 
[res.on.exception.handling]/5).

@ckennelly ping!


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,64 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+void *do_allocate(size_t, size_t)
+{ assert(false); }
+
+void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main(int, char**)
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+
+  return 0;
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,53 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+
+#include "count_new.hpp"
+
+void test(size_t initial_buffer_size)
+{
+

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 185229.
Quuxplusone added a subscriber: AlisdairM.
Quuxplusone added a comment.

Rebased on master. @ericwf (cc @AlisdairM) ping!


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+void *do_allocate(size_t, size_t)
+{ assert(false); }
+
+void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+
+#include "count_new.hpp"
+
+void test(size_t initial_buffer_size)
+{
+globalMemCounter.reset();
+
+std::experimental::pmr::monotonic_buffer_resource mono1(
+initial_buffer_size,
+ 

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

@EricWF ping!
and oops, I never submitted these comments, I guess.




Comment at: include/experimental/memory_resource:427
+{
+static const size_t __default_buffer_capacity = 1024;
+static const size_t __default_buffer_alignment = 16;

EricWF wrote:
> `constexpr` these constants if they're const?
Compare ``, which has "constexpr" in the synopsis comment but "static 
const" in the code.
However, I updated the diff to follow ``'s example and use `static 
_LIBCPP_CONSTEXPR const` everywhere.



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp:32
+void *ret = r1.allocate(2048, big_alignment);
+assert(ret != nullptr);
+assert(globalMemCounter.checkNewCalledEq(1));

EricWF wrote:
> This check isn't passing with ToT clang.
Agreed, it won't pass unless the underlying `new_delete_resource` supports 
overaligned alignments, which isn't true unless the underlying runtime supports 
aligned new. Which isn't true on my MacBook.

I think the fix would be for me to copy all the XFAIL lines from 
`test/libcxx/language.support/support.dynamic/libcpp_deallocate.sh.cpp`, is 
that right?


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 179718.
Quuxplusone added a comment.

Updated and addressed review comments.
`test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp`
 is now XFAILed on OSX platforms, using code copied-and-pasted from 
`test/libcxx/language.support/support.dynamic/libcpp_deallocate.sh.cpp`.

@EricWF ping!


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+void *do_allocate(size_t, size_t)
+{ assert(false); }
+
+void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 177540.
Quuxplusone marked 5 inline comments as done.
Quuxplusone added a comment.

@ericwf ping!


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+void *do_allocate(size_t, size_t)
+{ assert(false); }
+
+void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+
+#include "count_new.hpp"
+
+void test(size_t initial_buffer_size)
+{
+globalMemCounter.reset();
+
+std::experimental::pmr::monotonic_buffer_resource mono1(
+initial_buffer_size,
+

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF requested changes to this revision.
EricWF added a comment.
This revision now requires changes to proceed.

Some of the tests aren't passing with Trunk clang. Please fix these.




Comment at: include/experimental/memory_resource:427
+{
+static const size_t __default_buffer_capacity = 1024;
+static const size_t __default_buffer_alignment = 16;

`constexpr` these constants if they're const?



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp:32
+void *ret = r1.allocate(2048, big_alignment);
+assert(ret != nullptr);
+assert(globalMemCounter.checkNewCalledEq(1));

This check isn't passing with ToT clang.



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp:43
+assert(globalMemCounter.checkNewCalledEq(2));
+// assert(globalMemCounter.last_new_align >= 4);
+// assert(globalMemCounter.last_new_align < big_alignment);

Either remove this or uncomment them. Preferably the latter. More test coverage 
is better.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

@ericwf ping? This still needs someone to land it; I don't have commit privs. 
Thanks!


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

LGTM. The tests might need some XFAIL'ing, but otherwise I think this looks 
good.

Sorry for the wait.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47111/new/

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 173611.
Quuxplusone added a comment.

Rebased on master. @ericwf @ldionne ping please?


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+
+#include "count_new.hpp"
+
+void test(size_t initial_buffer_size)
+{
+globalMemCounter.reset();
+
+std::experimental::pmr::monotonic_buffer_resource mono1(
+initial_buffer_size,
+std::experimental::pmr::new_delete_resource()
+);
+

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

Long-delayed ping!


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 154486.
Quuxplusone added a comment.

Oops, my previous diff had a stray `)` in it, somehow.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+
+#include "count_new.hpp"
+
+void test(size_t initial_buffer_size)
+{
+globalMemCounter.reset();
+
+std::experimental::pmr::monotonic_buffer_resource mono1(
+initial_buffer_size,
+std::experimental::pmr::new_delete_resource()
+);
+

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D47111#1151714, @Quuxplusone wrote:

> > I'll take a pass at the tests tomorrow. Some pointers in general:
> > 
> > - Tests should be named after the component they test, not how they're 
> > testing it.
> > - All the tests for a single component should be in the same file.
>
> I'm certain I'm doing it fairly wrong and you'll have to give me specific 
> handholds like "merge these two tests into one file", "rename this file to 
> x.cpp", etc.
>  The current large number of files is only because *last* iteration, you told 
> me to split up the tests from "one big monotonic_buffer.pass.cpp" into one 
> file per test!


I'm sorry, I don't mean to jerk you around. I'm still learning to be a 
reviewer. I'll provide some specific examples to avoid the ambiguity from 
before.

> The problem with "one file per component" is that I don't understand what a 
> "component" is in this context. From last iteration, I know that "all of 
> `monotonic_buffer_resource`" is too big of a component! I also know that it's 
> impossible to test `deallocate` without also calling `allocate`, but the 
> reverse is not true.

Again, my apologies.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 154050.
Quuxplusone marked 2 inline comments as done.
Quuxplusone added a comment.

Massive changes based on Eric's feedback. Moved constructors and destructors of 
`monotonic_buffer_resource` into the header so they can be completely inlined. 
Renamed `struct __monotonic_buffer_foo_header` to nested types 
`monotonic_buffer_resource::__foo_header`. Refactored for 80-column lines.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone marked 7 inline comments as done.
Quuxplusone added a comment.

> I'll take a pass at the tests tomorrow. Some pointers in general:
> 
> - Tests should be named after the component they test, not how they're 
> testing it.
> - All the tests for a single component should be in the same file.

I'm certain I'm doing it fairly wrong and you'll have to give me specific 
handholds like "merge these two tests into one file", "rename this file to 
x.cpp", etc.
The current large number of files is only because *last* iteration, you told me 
to split up the tests from "one big monotonic_buffer.pass.cpp" into one file 
per test!
The problem with "one file per component" is that I don't understand what a 
"component" is in this context. From last iteration, I know that "all of 
`monotonic_buffer_resource`" is too big of a component! I also know that it's 
impossible to test `deallocate` without also calling `allocate`, but the 
reverse is not true.




Comment at: include/experimental/memory_resource:471
+
+void release();
+

EricWF wrote:
> I think it would be nice for `release` to be inline. What do you think?
Good point. It does pull a ton of stuff into the public header, but it's 
probably worth it.
I'm also moving the virtual destructor into the public header, so it can be 
inlined. (`do_allocate` becomes the new key function.)



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp:1
+// -*- C++ -*-
+//===--===//

EricWF wrote:
> We don't actually need this file, though older libc++ tests will often 
> include it.
> 
> It's only needed to keep empty directories around, but this one has children.
I was starting to wonder about this! Removed.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D47111#1114284, @Quuxplusone wrote:

> Refactor to remove unused fields from the header structs.
>
> Before this change, `sizeof(monotonic_buffer_resource) == 56` and the 
> overhead per allocated chunk was `40` bytes.
>  After this change, `sizeof(monotonic_buffer_resource) == 48` and the 
> overhead per allocated chunk is `24` bytes.


Woo! This is looking great!

I'm done looking at the implementation, I'll take a pass at the tests tomorrow. 
Some pointers in general:

1. Tests should be named after the component they test, not how they're testing 
it.
2. All the tests for a single component should be in the same file.
3. You can use `LIBCPP_ASSERT` to check specific implementation details, like 
exactly how much our allocation sizes grow.

(1) and (2) are important for maintaining the tests. (3) will probably be 
useful for actually writing meaningful tests despite
most observable behavior being implementation specific.




Comment at: include/experimental/memory_resource:425
+
+struct __monotonic_buffer_chunk_header;
+

Can these classes be members of `monotonic_buffer_resource`?



Comment at: include/experimental/memory_resource:451
+
+monotonic_buffer_resource(void* __buffer, size_t __buffer_size, 
memory_resource* __upstream);
+

Lets keep these constructors inline.



Comment at: include/experimental/memory_resource:467
+
+~monotonic_buffer_resource() override; // key function
+

Need _LIBCPP_FUNC_VIS



Comment at: include/experimental/memory_resource:471
+
+void release();
+

I think it would be nice for `release` to be inline. What do you think?



Comment at: include/experimental/memory_resource:489
+private:
+size_t __previous_allocation_size() const;
+

`__previous_allocation_size` isn't used in the header, so it shouldn't be 
declared there. We don't want to expose more symbols than we need to.



Comment at: src/experimental/memory_resource.cpp:212
+
+monotonic_buffer_resource::monotonic_buffer_resource(void* buffer, size_t 
buffer_size, memory_resource* upstream)
+: __res_(upstream)

In general, please wrap to 80 characters.



Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp:1
+// -*- C++ -*-
+//===--===//

We don't actually need this file, though older libc++ tests will often include 
it.

It's only needed to keep empty directories around, but this one has children.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 153881.

Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
@@ -0,0 +1,13 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+int main()
+{
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@
+//===--===//
+//
+// 

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 151983.
Quuxplusone added a comment.

Minor cosmetic changes.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
@@ -0,0 +1,13 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+int main()
+{
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: include/experimental/memory_resource:489
+memory_resource* __res_;
+size_t __next_buffer_size_;
+};

Quuxplusone wrote:
> I've discovered that Boost.Container does not bother to preserve this state 
> across calls to `release()`. If that's legal, then we can save 8 bytes here. 
> I've asked for an LWG issue to be opened on the subject of "what the heck is 
> `release()` supposed to do anyway."
On discussion with Ion Gaztañaga, Pablo Halpern, and Casey Carter, I've come to 
the conclusion that we *must not* preserve this state across calls to 
`release`, no matter what the Standard currently says, because that leads to 
insanely high memory usage when calling `mr.release()` in a loop. (Casey 
implies that MSVC's implementation has this issue; Pablo confirms that he never 
intended it.)

Fixed my implementation and added a regression test in 
`test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp`.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 151112.
Quuxplusone added a comment.
Herald added a subscriber: christof.

- Shrink monotonic_buffer_resource, and make release() not "leak" allocation 
size.

  Repeatedly calling allocate() and release() in a loop should not blow up. I 
originally thought this was required by the Standard, and maybe it is, but that 
was before I discovered this terrible effect, and before Pablo Halpern told me 
that it was not his actual intent.

  Since the only purpose of storing `__next_buffer_size_` in the object was to 
implement the "looping blows up" misfeature, we can now get rid of that field, 
which shrinks `monotonic_buffer_resource` from 56 bytes down to 48 bytes, 
matching Boost.Container's implementation.

- Add `REQUIRES: c++experimental` to all tests (oops).


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
@@ -0,0 +1,13 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+int main()
+{
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+ 

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: include/experimental/memory_resource:489
+memory_resource* __res_;
+size_t __next_buffer_size_;
+};

I've discovered that Boost.Container does not bother to preserve this state 
across calls to `release()`. If that's legal, then we can save 8 bytes here. 
I've asked for an LWG issue to be opened on the subject of "what the heck is 
`release()` supposed to do anyway."


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 148845.
Quuxplusone added a comment.

Refactor to remove unused fields from the header structs.

Before this change, `sizeof(monotonic_buffer_resource) == 56` and the overhead 
per allocated chunk was `40` bytes.
After this change, `sizeof(monotonic_buffer_resource) == 48` and the overhead 
per allocated chunk is `24` bytes.

Eric suggests replacing `size_t __align_` with `uint8_t __log2_align_`. I'm 
amenable to this idea, but I'd want to know what's the best way to compute the 
log2 of a user-supplied number.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
@@ -0,0 +1,13 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+int main()
+{
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,61 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
===
--- /dev/null
+++ 

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 148843.
Quuxplusone added a comment.

Add `override`; disintegrate the unit test; adopt some tests from Eric's 
https://reviews.llvm.org/D27402.
Also fix one QOI issue discovered by Eric's tests: if the user passes an 
`initial_size` to the constructor, then they are //probably// intending that 
our first upstream-allocation be big enough to serve at least `initial_size` 
1-byte allocations (or one `initial_size`-byte allocation with a suitably small 
alignment). This is explicitly //not// mandated by the Standard (it merely 
requires that our next upstream-allocation be of size at least `initial_size`), 
but it's probably a healthy choice.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
@@ -0,0 +1,13 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+int main()
+{
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,61 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: 

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: src/experimental/memory_resource.cpp:237
+
+void *result = __res_->allocate(aligned_capacity, align);
+__monotonic_buffer_header *header = (__monotonic_buffer_header *)((char 
*)result + aligned_capacity - header_size);

Quuxplusone wrote:
> For reference, here is where we ask the upstream for a block aligned 
> according to the user's `align`.
> It occurs to me that the upstream might not be able to satisfy such a request 
> (actually, `new_delete_resource` works that way for me because libc++ doesn't 
> support aligned new and delete on OSX), which would make the upstream throw 
> `bad_alloc`. We handle this by passing along the exception. We //could// 
> conceivably handle it by retrying with
> ```
> aligned_capacity += align;
> __res_->allocate(aligned_capacity, alignof(max_align_t));
> header->__alignment_ = alignof(max_align_t);
> ```
> but I'm not sure that that's any of `monotonic_buffer_resource`'s business. 
> Happy to make the patch if you think it ought to be.
I was initially thinking of storing `lg(align)`.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: include/experimental/memory_resource:428
+__monotonic_buffer_header *__next_;
+size_t __capacity_;
+size_t __alignment_;

Can't we determine the capacity in most cases simply using 
`static_cast(this) - static_cast(__start_)`?



Comment at: include/experimental/memory_resource:429
+size_t __capacity_;
+size_t __alignment_;
+size_t __used_;

Quuxplusone wrote:
> EricWF wrote:
> > I can't imagine we'll need more than 1 byte to represent the alignment.
> Even assuming for the sake of argument that that's right, it wouldn't buy us 
> anything here because of padding, would it?
> 
> At the moment, `__alignment_` needs to have enough range to store the `align` 
> parameter passed to `monotonic_buffer_resource::do_allocate`. In an SSE world 
> we should not blithely assume that `align < 256`. We //could// store 
> `lg(align)` in a single byte since `align` is always a power of two and less 
> than 2^64 — but then we're back to the first argument, which is that it'll be 
> padded to 8 bytes anyway so what do we gain.
> Even assuming for the sake of argument that that's right, it wouldn't buy us 
> anything here because of padding, would it?

It could. (A) we don't actually need to include the types trailing padding when 
tail allocating it as a part of a buffer.
and less importantly (B) I'm not sure all ABI's require the trailing padding of 
a type be included when that type is a data member of another type (I might 
just be wrong on this).

I'm also looking into other ways of improving the way your implementation packs 
data, which may cause this to be beneficial. 



Comment at: include/experimental/memory_resource:474
+protected:
+void* do_allocate(size_t __bytes, size_t __alignment);
+

Quuxplusone wrote:
> EricWF wrote:
> > Lets add `override` to these functions.
> I grepped for "override" in `include/` and saw exactly one (accidental?) use 
> in `experimental/filesystem`, so I thought probably libc++ had a policy not 
> to use it for portability reasons. I'll add it throughout, but would like 
> someone to explicitly confirm that
> 
> (A) it's okay to use in this header and wouldn't need to be guarded with a 
> `_LIBCPP_OVERRIDE` macro or anything
> 
> (B) Arthur should //not// bother trying to add it to any //other// headers, 
> not even "for consistency"
The header already assumes a full C++11 implementation (it uses `std::tuple`), 
the `override` keyword will bi available. No need for a special macro.




Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp:1
+//===--===//
+//

Instead of testing the whole of `monotonic_buffer` in a single file, this test 
should be broken up into separate files. Roughly one per function, or when it 
makes sense, one per subsection for the lowest level of heading (For example a 
single test for all constructors under `memory.buffer.ctor` ).


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: include/experimental/memory_resource:429
+size_t __capacity_;
+size_t __alignment_;
+size_t __used_;

EricWF wrote:
> I can't imagine we'll need more than 1 byte to represent the alignment.
Even assuming for the sake of argument that that's right, it wouldn't buy us 
anything here because of padding, would it?

At the moment, `__alignment_` needs to have enough range to store the `align` 
parameter passed to `monotonic_buffer_resource::do_allocate`. In an SSE world 
we should not blithely assume that `align < 256`. We //could// store 
`lg(align)` in a single byte since `align` is always a power of two and less 
than 2^64 — but then we're back to the first argument, which is that it'll be 
padded to 8 bytes anyway so what do we gain.



Comment at: include/experimental/memory_resource:474
+protected:
+void* do_allocate(size_t __bytes, size_t __alignment);
+

EricWF wrote:
> Lets add `override` to these functions.
I grepped for "override" in `include/` and saw exactly one (accidental?) use in 
`experimental/filesystem`, so I thought probably libc++ had a policy not to use 
it for portability reasons. I'll add it throughout, but would like someone to 
explicitly confirm that

(A) it's okay to use in this header and wouldn't need to be guarded with a 
`_LIBCPP_OVERRIDE` macro or anything

(B) Arthur should //not// bother trying to add it to any //other// headers, not 
even "for consistency"



Comment at: src/experimental/memory_resource.cpp:217
+{
+if (void *result = try_allocate_from_chunk(&__original_, bytes, align)) {
+return result;

EricWF wrote:
> Drop the braces for conditionals and loops with single statements.
*shiver* but okay.



Comment at: src/experimental/memory_resource.cpp:237
+
+void *result = __res_->allocate(aligned_capacity, align);
+__monotonic_buffer_header *header = (__monotonic_buffer_header *)((char 
*)result + aligned_capacity - header_size);

For reference, here is where we ask the upstream for a block aligned according 
to the user's `align`.
It occurs to me that the upstream might not be able to satisfy such a request 
(actually, `new_delete_resource` works that way for me because libc++ doesn't 
support aligned new and delete on OSX), which would make the upstream throw 
`bad_alloc`. We handle this by passing along the exception. We //could// 
conceivably handle it by retrying with
```
aligned_capacity += align;
__res_->allocate(aligned_capacity, alignof(max_align_t));
header->__alignment_ = alignof(max_align_t);
```
but I'm not sure that that's any of `monotonic_buffer_resource`'s business. 
Happy to make the patch if you think it ought to be.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

FYI I have a full implementation of this laying around as 
https://reviews.llvm.org/D27402 (https://reviews.llvm.org/D27402). But I have 
never taken the time to resolve merge conflicts.
Feel free to steal any of the tests if they're still relevant.




Comment at: include/experimental/memory_resource:429
+size_t __capacity_;
+size_t __alignment_;
+size_t __used_;

I can't imagine we'll need more than 1 byte to represent the alignment.



Comment at: include/experimental/memory_resource:474
+protected:
+void* do_allocate(size_t __bytes, size_t __alignment);
+

Lets add `override` to these functions.



Comment at: src/experimental/memory_resource.cpp:217
+{
+if (void *result = try_allocate_from_chunk(&__original_, bytes, align)) {
+return result;

Drop the braces for conditionals and loops with single statements.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 148542.
Quuxplusone retitled this revision from "Implement monotonic_buffer_resource in 
" to ": Implement 
monotonic_buffer_resource.".
Quuxplusone added 1 blocking reviewer(s): EricWF.
Quuxplusone added a comment.

Fix one visibility macro.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp
@@ -0,0 +1,408 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+#include "count_new.hpp"
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+struct repointable_resource : public std::experimental::pmr::memory_resource
+{
+std::experimental::pmr::memory_resource *which;
+
+explicit repointable_resource(std::experimental::pmr::memory_resource *res) : which(res) {}
+
+protected:
+virtual void *do_allocate(size_t size, size_t align)
+{ return which->allocate(size, align); }
+
+virtual void do_deallocate(void *p, size_t size, size_t align)
+{ return which->deallocate(p, size, align); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const ) const noexcept
+{ return which->is_equal(rhs); }
+};
+
+void test_construction_with_default_resource()
+{
+std::experimental::pmr::memory_resource *expected = std::experimental::pmr::null_memory_resource();
+std::experimental::pmr::set_default_resource(expected);
+{
+char buffer[16];
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2(16);
+std::experimental::pmr::monotonic_buffer_resource r3(buffer, sizeof buffer);
+assert(r1.upstream_resource() == expected);
+assert(r2.upstream_resource() == expected);
+assert(r3.upstream_resource() == expected);
+}
+
+expected = std::experimental::pmr::new_delete_resource();
+std::experimental::pmr::set_default_resource(expected);
+{
+char buffer[16];
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2(16);
+std::experimental::pmr::monotonic_buffer_resource r3(buffer, sizeof buffer);
+assert(r1.upstream_resource() == expected);
+assert(r2.upstream_resource() == expected);
+assert(r3.upstream_resource() == expected);
+}
+}
+
+void test_construction_without_buffer()
+{
+// Constructing a monotonic_buffer_resource should not cause allocations
+// by itself; the resource should wait to allocate until an allocation is
+// requested.
+
+globalMemCounter.reset();
+std::experimental::pmr::set_default_resource(std::experimental::pmr::new_delete_resource());
+
+std::experimental::pmr::monotonic_buffer_resource r1;
+assert(globalMemCounter.checkNewCalledEq(0));
+
+std::experimental::pmr::monotonic_buffer_resource r2(1024);
+assert(globalMemCounter.checkNewCalledEq(0));
+
+std::experimental::pmr::monotonic_buffer_resource r3(1024, std::experimental::pmr::new_delete_resource());
+assert(globalMemCounter.checkNewCalledEq(0));
+}
+
+void test_equality()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+

[PATCH] D47111: Implement monotonic_buffer_resource in

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone created this revision.
Quuxplusone added a reviewer: EricWF.
Herald added a subscriber: cfe-commits.

(https://reviews.llvm.org/D47090 included this in ``; at 
Eric's request, I've split this out into its own patch applied to the existing 
`` instead.)


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp
@@ -0,0 +1,408 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+#include "count_new.hpp"
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+struct repointable_resource : public std::experimental::pmr::memory_resource
+{
+std::experimental::pmr::memory_resource *which;
+
+explicit repointable_resource(std::experimental::pmr::memory_resource *res) : which(res) {}
+
+protected:
+virtual void *do_allocate(size_t size, size_t align)
+{ return which->allocate(size, align); }
+
+virtual void do_deallocate(void *p, size_t size, size_t align)
+{ return which->deallocate(p, size, align); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const ) const noexcept
+{ return which->is_equal(rhs); }
+};
+
+void test_construction_with_default_resource()
+{
+std::experimental::pmr::memory_resource *expected = std::experimental::pmr::null_memory_resource();
+std::experimental::pmr::set_default_resource(expected);
+{
+char buffer[16];
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2(16);
+std::experimental::pmr::monotonic_buffer_resource r3(buffer, sizeof buffer);
+assert(r1.upstream_resource() == expected);
+assert(r2.upstream_resource() == expected);
+assert(r3.upstream_resource() == expected);
+}
+
+expected = std::experimental::pmr::new_delete_resource();
+std::experimental::pmr::set_default_resource(expected);
+{
+char buffer[16];
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2(16);
+std::experimental::pmr::monotonic_buffer_resource r3(buffer, sizeof buffer);
+assert(r1.upstream_resource() == expected);
+assert(r2.upstream_resource() == expected);
+assert(r3.upstream_resource() == expected);
+}
+}
+
+void test_construction_without_buffer()
+{
+// Constructing a monotonic_buffer_resource should not cause allocations
+// by itself; the resource should wait to allocate until an allocation is
+// requested.
+
+globalMemCounter.reset();
+std::experimental::pmr::set_default_resource(std::experimental::pmr::new_delete_resource());
+
+std::experimental::pmr::monotonic_buffer_resource r1;
+assert(globalMemCounter.checkNewCalledEq(0));
+
+std::experimental::pmr::monotonic_buffer_resource r2(1024);
+assert(globalMemCounter.checkNewCalledEq(0));
+
+std::experimental::pmr::monotonic_buffer_resource r3(1024, std::experimental::pmr::new_delete_resource());
+assert(globalMemCounter.checkNewCalledEq(0));
+}
+
+void test_equality()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+