[PATCH] D47111: : Implement monotonic_buffer_resource.

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/ne

[PATCH] D47111: : Implement monotonic_buffer_resource.

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 ___

[PATCH] D47111: : Implement monotonic_buffer_resource.

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_alignmen

[PATCH] D47111: : Implement monotonic_buffer_resource.

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 thi

[PATCH] D47111: : Implement monotonic_buffer_resource.

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 `__defaul

[PATCH] D47111: : Implement monotonic_buffer_resource.

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/mem

[PATCH] D47111: : Implement monotonic_buffer_resource.

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 AC

[PATCH] D47111: : Implement monotonic_buffer_resource.

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

[PATCH] D47111: : Implement monotonic_buffer_resource.

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; -

[PATCH] D47111: : Implement monotonic_buffer_resource.

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-p

[PATCH] D47111: : Implement monotonic_buffer_resource.

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/m

[PATCH] D47111: : Implement monotonic_buffer_resource.

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_buff

[PATCH] D47111: : Implement monotonic_buffer_resource.

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-

[PATCH] D47111: : Implement monotonic_buffer_resource.

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/n

[PATCH] D47111: : Implement monotonic_buffer_resource.

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/experime

[PATCH] D47111: : Implement monotonic_buffer_resource.

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.

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/ex

[PATCH] D47111: : Implement monotonic_buffer_resource.

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

[PATCH] D47111: : Implement monotonic_buffer_resource.

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 `str

[PATCH] D47111: : Implement monotonic_buffer_resource.

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 f

[PATCH] D47111: : Implement monotonic_buffer_resource.

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, `size

[PATCH] D47111: : Implement monotonic_buffer_resource.

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_ge

[PATCH] D47111: : Implement monotonic_buffer_resource.

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.resour

[PATCH] D47111: : Implement monotonic_buffer_resource.

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 c

[PATCH] D47111: : Implement monotonic_buffer_resource.

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

[PATCH] D47111: : Implement monotonic_buffer_resource.

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

[PATCH] D47111: : Implement monotonic_buffer_resource.

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_

[PATCH] D47111: : Implement monotonic_buffer_resource.

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, the

[PATCH] D47111: : Implement monotonic_buffer_resource.

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); --

[PATCH] D47111: : Implement monotonic_buffer_resource.

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_

[PATCH] D47111: : Implement monotonic_buffer_resource.

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 f

[PATCH] D47111: : Implement monotonic_buffer_resource.

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. Comm

[PATCH] D47111: : Implement monotonic_buffer_resource.

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:

[PATCH] D47111: Implement monotonic_buffer_resource in

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