Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mem.res.pool.overview] Implicit definition of terms #1258

Open
jensmaurer opened this issue Dec 15, 2016 · 8 comments · May be fixed by #1964
Open

[mem.res.pool.overview] Implicit definition of terms #1258

jensmaurer opened this issue Dec 15, 2016 · 8 comments · May be fixed by #1964
Assignees

Comments

@jensmaurer
Copy link
Member

The terms "pools", "chunks", upstream memory resource", "upstream allocator" are proclaimed to be defined here, but the phrasing doesn't actually say "A pool is ..." etc.

@tkoeppe
Copy link
Contributor

tkoeppe commented Feb 5, 2017

@AlisdairM: Could you please take a look?

@AlisdairM
Copy link
Contributor

I'll try to get to this in the next week or so - we will want a resolution for Kona.

@AlisdairM
Copy link
Contributor

Acknowledging that this still needs attention - will try to pull together some feedback before leaving Kona Saturday night.

@jwakely
Copy link
Member

jwakely commented Nov 24, 2017

Currently [mem.res.pool.options] has a cross-reference to [mem.res.monotonic.buffer] for the term "upstream memory resource" (added editorially in 7f9d7f7) which is completely bogus - there is no relationship between pool resources and monotonic buffers. The cross-reference should go to [mem.res.pool.overview], unfortunately that defines "upstream allocator" not "upstream memory resource" (which is also bogus - it's not an allocator, and the member function to obtain it is upstream_resource())

I suggest changing [mem.res.pool.overview] bullet (1.3) to define "upstream memory resource" and changing the reference in [mem.res.pool.options] to point there instead.

[mem.res.pool.overview] defines pools and chunks with defn but then uses "blocks" as a normal word, not a defn, even though we refer to blocks a lot more than we refer to chunks (e.g. in [mem.res.pool.options] and [mem.res.pool.mem]) ... and then again in [mem.res.monotonic.buffer] with no context at all). The descriptions for operator delete in [new.delete.single] and [new.delete.array] also talk about blocks, despite operator new not doing so. Maybe the lib intro should define "allocated block" as a term, and then we can use that consistently in the library clauses. "blocks" on its own is not a good term, because [defns.block] says it's something entirely different.

@jwakely
Copy link
Member

jwakely commented Nov 24, 2017

Here's a first stab at rewording this to define the terms properly, and change "upstream allocator" to "upstream memory resource":


The synchronized_pool_resource and unsynchronized_pool_resource classes (collectively called pool resource classes) are general-purpose memory resources having the following qualities:

  • A pool resource has an upstream memory resource, supplied on construction, which is used to allocate memory that the pool resource subdivides to satisfy calls to its own do_allocate member.
  • A chunk is a region of storage allocated from the upstream memory resource. A pool resource divides each chunk into one or more blocks of the same block size. Different chunks can have different block sizes, and different numbers of blocks.
  • A pool is a collection of chunks that all have the same block size. Each pool resource contains a collection of pools with different block sizes.
  • On destruction, a pool resource frees all the memory it obtained from its upstream memory resource by calling the upstream resource's deallocate function. [Note: This invalidates all memory returned by the pool resource's do_allocate function, even if do_deallocate has not been called for some of the allocated blocks. --end note]
  • Each call to do_allocate(size, alignment) is dispatched to the pool serving the smallest blocks accommodating at least size bytes.
  • When a particular pool is exhausted, allocating a block from that pool results in the allocation of an additional chunk from the upstream memory resource, thus replenishing the pool. With each successive replenishment, the chunk size obtained increases geometrically. [Note: By allocating memory in chunks, the pooling strategy increases the chance that consecutive allocations will be close together in memory. --end note]
  • Allocation requests that exceed the largest block size of any pool are fulfilled directly from the upstream allocator.
  • A pool_options struct may be passed to the pool resource constructors to tune the largest block size and the maximum chunk size.

A synchronized_pool_resource may be accessed from multiple threads without external synchronization and may have thread-specific pools to reduce synchronization costs. An unsynchronized_pool_resource class may not be accessed from multiple threads simultaneously and thus avoids the cost of synchronization entirely in single-threaded applications.


Should the "Each call to do_allocate(size, alignment) ..." and "Allocation requests that exceed..." bullets take alignment into account? A call to do_allocate(4, 4096) will probably be passed straight upstream, rather than trying to use the pool for block size 4.

@CaseyCarter
Copy link
Contributor

Should the "Each call to do_allocate(size, alignment) ..." and "Allocation requests that exceed..." bullets take alignment into account? A call to do_allocate(4, 4096) will probably be passed straight upstream, rather than trying to use the pool for block size 4.

I'd say yes: in our implementation, do_allocate(4, 4096) allocates a block from the pool for block size 4096. I propose:

  • Each call to do_allocate(size, alignment) is dispatched to the pool serving the smallest blocks that accommodate size bytes and are aligned to a multiple of alignment.

[...]

  • Allocation requests that exceed the largest block size and/or alignment of any pool are fulfilled directly from the upstream allocator.

@jensmaurer
Copy link
Member Author

@jwakely, I'd suggest you turn your proposal into a pull request; we might want to have an LWG check on the change.

@jensmaurer jensmaurer added the decision-required A decision of the editorial group (or the Project Editor) is required. label Feb 14, 2018
@jensmaurer
Copy link
Member Author

@zygoloid agrees on the proposed course of action.

@jensmaurer jensmaurer removed the decision-required A decision of the editorial group (or the Project Editor) is required. label Mar 18, 2018
jwakely added a commit to jwakely/draft that referenced this issue Mar 21, 2018
Update cross-reference in [mem.res.pool.options] to refer to
[mem.res.pool.overview].

Fixes cplusplus#1258
jwakely added a commit to jwakely/draft that referenced this issue Aug 15, 2018
Update cross-reference in [mem.res.pool.options] to refer to
[mem.res.pool.overview].

Fixes cplusplus#1258
jwakely added a commit to jwakely/draft that referenced this issue Apr 20, 2022
Update cross-reference in [mem.res.pool.options] to refer to
[mem.res.pool.overview].

Fixes cplusplus#1258
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants