cplusplus / draft

C++ standards drafts
http://www.open-std.org/jtc1/sc22/wg21/
5.66k stars 748 forks source link

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

Open jensmaurer opened 7 years ago

jensmaurer commented 7 years ago

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 commented 7 years ago

@AlisdairM: Could you please take a look?

AlisdairM commented 7 years ago

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

AlisdairM commented 7 years ago

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

jwakely commented 6 years ago

Currently [mem.res.pool.options] has a cross-reference to [mem.res.monotonic.buffer] for the term "upstream memory resource" (added editorially in 7f9d7f759df375a916e312cb41f194397c7754c9) 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 commented 6 years ago

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 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 commented 6 years ago

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 commented 6 years ago

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

jensmaurer commented 6 years ago

@zygoloid agrees on the proposed course of action.