From 4d8378663c9e610df5eb25f4301c6d9d15e07721 Mon Sep 17 00:00:00 2001 From: Daniel Polo <106583643+danielPoloWork@users.noreply.github.com> Date: Tue, 7 Jul 2026 19:42:48 +0200 Subject: [PATCH] docs(spec): reconcile the memory-pool spec with the as-built system The spec was the original greenfield brief and had drifted from the delivered, v1.0.0-frozen library. Reconcile sections 1-6 with the implementation and cross-link the realizing ADRs; formalize the section 2/3 subsection anchors already referenced across the ADR set; add a section 7 mapping the spec to its decision records and listing the explicitly deferred items. - 1: state the external-vs-internal fragmentation scope precisely - 2.2: disambiguate dynamic growth as non-contiguous chunk-linking with pointer-validity guarantees (ADR-0022/0023/0024) - 2.4: document the mutex/lock-free policy knob + ABA-tagged head (ADR-0020) - 4.1: add block-size/alignment/strict-aliasing constraints (ADR-0009) - 5: split C core / C++17 surface / error semantics / introspection, documenting double-free/foreign/NULL behaviour (ADR-0012/0016) and the InstrumentedPool introspection surface (ADR-0015/0025) - 6: point the benchmark at ADR-0014, note the contended scenario, extend the Valgrind criterion, add the sanitizer/CI tier - 7: spec->ADR map + deferred items (#107 pmr, #108 fuzz, #109 hardening) Mark the zh-Hans/ja spec translation rows stale in the i18n manifest (the established dance when an English source changes; re-sync follows in a separate PR). CHANGELOG Unreleased updated. Refs #105. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 11 ++ docs/i18n/translation-status.md | 4 +- docs/specs/01_spec_cpp_memory_pool.md | 156 ++++++++++++++++++++++---- 3 files changed, 147 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7956571..a768cd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,17 @@ dated version block (`## [X.Y.Z] — YYYY-MM-DD`) when a release PR closes a mil `translation-status.md` manifest's two README rows are re-pinned to the current source commit (`d38b598`) and flipped from `stale` back to `translated`, clearing the `i18n-freshness` flag the `v1.1.2` release raised. Documentation-only; no API change. +- **Specification reconciled with the as-built system.** + [`docs/specs/01_spec_cpp_memory_pool.md`](docs/specs/01_spec_cpp_memory_pool.md) was the + original greenfield brief and had drifted from the delivered (`v1.0.0`-frozen) library. It + now cross-links the realizing ADRs, formalizes the `§2`/`§3` subsection anchors already + referenced across the ADR set, disambiguates the `§2.2` dynamic-growth model (non-contiguous + chunk-linking; ADR-0022/0023/0024), documents the `§4.1` + block-size/alignment/strict-aliasing constraints (ADR-0009), the `§5.3` error semantics + (ADR-0012/0016) and `§5.4` introspection (ADR-0015/0025), and adds a `§7` spec→ADR map plus + the explicitly deferred items (#107 `pmr`, #108 fuzzing, #109 hardening). The + `zh-Hans`/`ja` spec translation rows are marked `stale` pending a follow-up re-sync. + Documentation-only; no API change. Refs #105. ## Released versions diff --git a/docs/i18n/translation-status.md b/docs/i18n/translation-status.md index 8eb6056..226d9e9 100644 --- a/docs/i18n/translation-status.md +++ b/docs/i18n/translation-status.md @@ -17,7 +17,7 @@ Status vocabulary: | Source page | Source commit | Translated at | Status | Reviewer | |-------------|:-------------:|:-------------:|:------:|----------| | [`README.md`](../../README.md) | `d38b598` | `d38b598` | `translated` | — | -| [`docs/specs/01_spec_cpp_memory_pool.md`](../specs/01_spec_cpp_memory_pool.md) | `2e55dfa` | `2e55dfa` | `translated` | — | +| [`docs/specs/01_spec_cpp_memory_pool.md`](../specs/01_spec_cpp_memory_pool.md) | `2e55dfa` | `2e55dfa` | `stale` | — | | [`docs/patterns/README.md`](../patterns/README.md) | `524f0cc` | `524f0cc` | `translated` | — | ## `ja` (Japanese) @@ -25,7 +25,7 @@ Status vocabulary: | Source page | Source commit | Translated at | Status | Reviewer | |-------------|:-------------:|:-------------:|:------:|----------| | [`README.md`](../../README.md) | `d38b598` | `d38b598` | `translated` | — | -| [`docs/specs/01_spec_cpp_memory_pool.md`](../specs/01_spec_cpp_memory_pool.md) | `612f9d2` | `612f9d2` | `translated` | — | +| [`docs/specs/01_spec_cpp_memory_pool.md`](../specs/01_spec_cpp_memory_pool.md) | `612f9d2` | `612f9d2` | `stale` | — | | [`docs/patterns/README.md`](../patterns/README.md) | `6c6aeb7` | `6c6aeb7` | `translated` | — | > Seeded by ROADMAP §8.2 with the full translatable surface at `missing`. The diff --git a/docs/specs/01_spec_cpp_memory_pool.md b/docs/specs/01_spec_cpp_memory_pool.md index 65bd5b4..03dc7df 100644 --- a/docs/specs/01_spec_cpp_memory_pool.md +++ b/docs/specs/01_spec_cpp_memory_pool.md @@ -1,32 +1,69 @@ # Software Specification: Purpose-built reference High-Performance Memory Pool Manager (C/C++) +> **Status — reconciled with the as-built system.** This document was originally a +> short greenfield brief. The library has since shipped (public API frozen at +> `v1.0.0`) and every non-trivial decision is recorded in an +> [Architecture Decision Record](../adr/). Each requirement below now cross-links the +> ADR(s) that realize it; [Section 7](#7-implementation-status--decision-records) maps +> the whole specification to its ADRs and lists the explicitly deferred items. + ## 1. Objective & Business Context Many high-performance systems (e.g. graphics engines, financial trading servers, databases) suffer from memory fragmentation and the overhead generated by frequent `malloc`/`free` or `new`/`delete` calls. -This component aims to provide a **custom memory allocator** (Memory Pool) that pre-allocates a contiguous block of memory, handling the allocation and deallocation of fixed-size blocks in constant time $O(1)$ with zero external fragmentation. +This component provides a **custom memory allocator** (Memory Pool) that pre-allocates a contiguous block of memory, handling the allocation and deallocation of fixed-size blocks in constant time $O(1)$ with zero **external** fragmentation. + +**Fragmentation scope (precise claim).** Fixed-size blocks eliminate *external* fragmentation *within a pool*: any free block satisfies any request, so free memory never degrades into unusable holes. The pool does **not** eliminate *internal* fragmentation — a caller that stores an object smaller than `block_size` wastes the difference. Choosing `block_size` to match the dominant object size is the caller's responsibility; the pool trades internal fragmentation for $O(1)$ determinism and zero external fragmentation by design. --- ## 2. Functional Requirements -- **Initialization:** The system must be able to pre-allocate a contiguous memory pool, specifying the block size (`block_size`) and the maximum number of blocks (`block_count`). -- **Allocation ($O(1)$):** It must return a pointer to a free block of the pool in constant time. If the pool is exhausted, it must return `NULL` (or throw an exception in C++), or request a new contiguous block if configured in dynamic mode. -- **Deallocation ($O(1)$):** It must mark a previously allocated block as available again in constant time, without returning it to the operating system immediately. -- **Thread Safety:** The pool must support concurrent access by multiple threads without memory corruption (optional, or configurable via a compile-time macro to maximize single-thread performance). +### 2.1 Initialization + +The system pre-allocates a memory pool, specifying the block size (`block_size`) and the maximum number of blocks (`block_count`). The `block_size * block_count` product must not overflow `size_t`; the constructor rejects overflow and degenerate arguments. See [ADR-0009](../adr/0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md). + +### 2.2 Allocation ($O(1)$) + +Return a pointer to a free block in constant time. When the pool is exhausted the behaviour depends on the growth mode: + +- **Fixed pool (default):** return `NULL` (C) or throw (C++ wrapper, [ADR-0016](../adr/0016-exception-policy-at-the-c-cpp-boundary.md)). +- **Dynamic pool:** acquire a **new, separate contiguous chunk** and satisfy the request from it. The growth strategy is **non-contiguous chunked expansion via a linked chunk list** — the pool is *not* a single contiguous region once it has grown, and it never relocates: **every previously returned pointer stays valid across growth** (no `realloc`-style invalidation). Growth is geometric (a configurable factor). Consequently, pointer-range validation (Section 6.1) is a **multi-chunk** membership test, not a single range check. This resolves the original brief's ambiguity in favour of chunk-linking over address-space reserve-and-commit. See [ADR-0022](../adr/0022-dynamic-growth-policy-and-chunk-linking.md), [ADR-0023](../adr/0023-composite-chunk-list-representation.md), [ADR-0024](../adr/0024-dynamic-growth-synchronization-and-creation-surface.md). + +### 2.3 Deallocation ($O(1)$) + +Mark a previously allocated block available again in constant time, without returning it to the operating system immediately. Freeing `NULL` or a pointer foreign to the pool is a defined, silent no-op; see [Section 5.3](#53-error-semantics) and [ADR-0012](../adr/0012-foreign-pointer-and-out-of-range-pointer-policy.md). + +### 2.4 Thread Safety + +Concurrent access is governed by a **compile-time policy knob** (`PBR_MEMORY_POOL_THREAD_SAFETY`) selecting one of three strategies, so single-thread users pay nothing: + +- `NONE` — single-threaded fast path (intentionally racy; never share such a pool across threads); +- `MUTEX` — a mutex guards the free list; +- `LOCKFREE` — a Treiber-stack free list whose head is an **ABA-tagged pointer** (`{pointer, tag}`, the tag incremented on every CAS) so the classic ABA hazard on a LIFO free list cannot mis-link. + +See [ADR-0020](../adr/0020-thread-safety-strategy-and-compile-time-knob.md). A contended, multi-thread benchmark exists (Section 6.3). --- ## 3. Non-Functional Requirements -- **No Memory Leaks:** When the pool is destroyed, all pre-allocated memory must be returned to the operating system. -- **Minimal Memory Overhead:** The use of internal metadata to track free blocks (e.g. via an internal linked list or a bitmask) must be minimal. -- **Compatibility:** Written in standard ANSI C (or C++17) with no external dependencies. +### 3.1 No Memory Leaks + +When the pool is destroyed, all pre-allocated memory (every chunk, for a grown pool) is returned to the operating system. Verified under Valgrind and the sanitizers (Section 6). + +### 3.2 Minimal Memory Overhead + +Free blocks carry **zero** per-block metadata — the free-list next-pointer is stored *in-band* inside the free block (Section 4). The only fixed cost is one pool-header struct plus, for dynamic pools, one small descriptor per chunk. The per-pool metadata budget is quantified and guarded in [ADR-0015](../adr/0015-metadata-overhead-budget-and-introspection.md). + +### 3.3 Compatibility + +Written to standard **ANSI C ABI** (the four-function surface) with an idiomatic **C++17** wrapper layer, no external runtime dependencies. Toolchain matrix and supported platforms: [ADR-0005](../adr/0005-toolchain-matrix-and-supported-platforms.md). --- ## 4. Logical Architecture & Algorithm (Free List) -The pool manages free memory using a **Free List** implicit within the blocks themselves. When a block is free, its first bytes are used to store a pointer to the next free block. This zeroes the metadata overhead for unused blocks. +The pool manages free memory using a **Free List** implicit within the blocks themselves. When a block is free, its first bytes store a pointer to the next free block. This zeroes the metadata overhead for unused blocks. ```text +-------------------------------------------------------------------+ @@ -39,38 +76,113 @@ The pool manages free memory using a **Free List** implicit within the blocks th +-------------------------------------------------------------------+ ``` +### 4.1 Constraints & Guarantees + +Storing a next-pointer in-band imposes constraints, all enforced by the implementation ([ADR-0009](../adr/0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md)): + +- **Minimum block size:** `block_size >= sizeof(void*)` — the constructor rejects smaller sizes. +- **Alignment:** every returned pointer is aligned to `alignof(std::max_align_t)`, uniformly. A weaker or caller-specified per-pool alignment was considered and **deliberately rejected** (the frozen C signature carries no alignment parameter; uniform strong alignment keeps `TypedPool` a trivial cast). +- **Strict-aliasing-safe access:** the in-band pointer is read/written through the canonical `*static_cast(slot)` idiom, not a type-punning cast of the user type. + +The intrusive-free-list design was chosen over a bitmap allocator; the rejected alternative and its rationale are recorded in [ADR-0009](../adr/0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md). + --- -## 5. API / Public Interface (C) +## 5. API / Public Interface + +### 5.1 C core (frozen at `v1.0.0`) ```c typedef struct memory_pool memory_pool_t; -// Initialize the memory pool +// Initialize a fixed-capacity memory pool memory_pool_t* memory_pool_create(size_t block_size, size_t block_count); -// Allocate a block from the pool (O(1)) +// Initialize a dynamically-growing pool (geometric chunk expansion; ADR-0022) +memory_pool_t* memory_pool_create_dynamic(size_t block_size, size_t block_count, + size_t growth_factor); + +// Allocate a block from the pool (O(1)); NULL when a fixed pool is exhausted void* memory_pool_alloc(memory_pool_t* pool); // Release a block back into the pool (O(1)) void memory_pool_free(memory_pool_t* pool, void* block); -// Destroy the pool, releasing all memory back to the operating system +// Destroy the pool, releasing all memory (every chunk) back to the OS void memory_pool_destroy(memory_pool_t* pool); ``` +The opaque `memory_pool_t` handle hides the implementation (Pimpl across the C/C++ boundary, [ADR-0010](../adr/0010-raii-pool-wrapper-and-pimpl-across-the-c-cpp-boundary.md)). + +### 5.2 C++17 surface + +A layered, idiomatic C++ surface built on the C core: + +- `Pool` — move-only RAII owner; `PoolBuilder` / factory methods for construction ([ADR-0010](../adr/0010-raii-pool-wrapper-and-pimpl-across-the-c-cpp-boundary.md), [ADR-0011](../adr/0011-factory-method-and-builder-for-pool-construction.md)). +- `TypedPool` — type-safe `construct`/`destroy` over the pool ([ADR-0017](../adr/0017-typed-pool-design.md)). +- `PoolAllocator` — an STL `Allocator` adapter so the pool can back `std::list`, `std::map`, … ([ADR-0018](../adr/0018-stl-allocator-adapter.md)). +- `InstrumentedPool` — a Decorator adding counters and lifecycle Observers ([ADR-0025](../adr/0025-decorator-for-instrumented-pool.md), [ADR-0026](../adr/0026-observer-for-pool-lifecycle-events.md)). + +### 5.3 Error semantics + +- **Freeing `NULL`** — no-op (mirrors C `free(NULL)`). +- **Freeing a foreign / out-of-range pointer** — defined, silent no-op detected in $O(1)$; no corruption ([ADR-0012](../adr/0012-foreign-pointer-and-out-of-range-pointer-policy.md)). +- **Double-free** — a double-free of an in-range, currently-free block is **not** detected by the default build (accepted trade-off, documented in [ADR-0012](../adr/0012-foreign-pointer-and-out-of-range-pointer-policy.md)); opt-in detection is tracked as future hardening (Section 7). +- **C++ exhaustion** — throws per [ADR-0016](../adr/0016-exception-policy-at-the-c-cpp-boundary.md); exceptions never cross the C ABI. + +### 5.4 Introspection + +Observability is provided two ways: an optional debug diagnostics surface (compiled out of release, gated by `PBR_MEMORY_POOL_DIAGNOSTICS`) and the production-usable `InstrumentedPool` decorator, which exposes live-block count, capacity, and a high-water mark ([ADR-0015](../adr/0015-metadata-overhead-budget-and-introspection.md), [ADR-0025](../adr/0025-decorator-for-instrumented-pool.md)). + --- ## 6. Verification & Test Strategy -1. **Correctness Tests:** Allocate all blocks until exhaustion; verify the behaviour with null inputs or pointers foreign to the pool. -2. **Leak Verification (Valgrind):** - - Verification command: +### 6.1 Correctness Tests + +Allocate all blocks until exhaustion; verify behaviour with null inputs and pointers foreign to the pool (defined no-op, Section 5.3). For dynamic pools, verify that pointers remain valid across a growth event and that foreign-pointer validation is a correct multi-chunk membership test. + +### 6.2 Leak Verification (Valgrind) + +```bash +gcc -g -O0 test_pool.c memory_pool.c -o test_pool +valgrind --leak-check=full --show-leak-kinds=all ./test_pool +``` + +**Success criterion:** `ERROR SUMMARY: 0 errors from 0 contexts` **and** `definitely lost: 0 bytes in 0 blocks`. + +### 6.3 Performance Benchmark + +`memory_pool_alloc`/`free` are compared against standard `malloc`/`free`. The methodology is fixed by [ADR-0014](../adr/0014-microbenchmark-methodology-pool-vs-malloc.md): warm-up repeat discarded, min/median/mean/max/stddev reported (not a single wall-clock loop), anti-optimization barriers, disclosed compiler flags and host, per-release reports under `docs/bench/`, and a non-asserting CI smoke run. A **concurrent** scenario (T threads on a shared pool, `MUTEX` vs `LOCKFREE`) provides the contended baseline. + +### 6.4 Sanitizers & CI + +ASan, UBSan and TSan run via dedicated CMake presets; TSan covers the thread-safe configurations. All of the above is wired into a multi-OS CI matrix (warnings-as-errors, `clang-tidy`, Valgrind). A coverage-guided fuzz target is deferred (Section 7). + +--- + +## 7. Implementation Status & Decision Records + +Every requirement above is realized and recorded. The table maps the spec to its ADRs. + +| Spec area | Realized by | +|-----------|-------------| +| §2.2 growth model | [ADR-0022](../adr/0022-dynamic-growth-policy-and-chunk-linking.md), [ADR-0023](../adr/0023-composite-chunk-list-representation.md), [ADR-0024](../adr/0024-dynamic-growth-synchronization-and-creation-surface.md) | +| §2.4 thread safety (mutex / lock-free + ABA tag) | [ADR-0020](../adr/0020-thread-safety-strategy-and-compile-time-knob.md) | +| §3.2 overhead budget & introspection | [ADR-0015](../adr/0015-metadata-overhead-budget-and-introspection.md) | +| §4 free-list layout, constraints, alignment, intrusive-vs-bitmap | [ADR-0009](../adr/0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md) | +| §5.1–5.2 API, RAII, Pimpl, builder, typed pool, STL adapter | [ADR-0010](../adr/0010-raii-pool-wrapper-and-pimpl-across-the-c-cpp-boundary.md), [ADR-0011](../adr/0011-factory-method-and-builder-for-pool-construction.md), [ADR-0017](../adr/0017-typed-pool-design.md), [ADR-0018](../adr/0018-stl-allocator-adapter.md) | +| §5.3 error semantics | [ADR-0012](../adr/0012-foreign-pointer-and-out-of-range-pointer-policy.md), [ADR-0016](../adr/0016-exception-policy-at-the-c-cpp-boundary.md) | +| §5.4 instrumentation / observers | [ADR-0025](../adr/0025-decorator-for-instrumented-pool.md), [ADR-0026](../adr/0026-observer-for-pool-lifecycle-events.md) | +| §6.3 benchmark methodology | [ADR-0014](../adr/0014-microbenchmark-methodology-pool-vs-malloc.md) | +| Spec-compliance acceptance audit | [ADR-0029](../adr/0029-spec-compliance-acceptance-audit.md) | + +### 7.1 Deferred / tracked - ```bash - gcc -g -O0 test_pool.c memory_pool.c -o test_pool - valgrind --leak-check=full --show-leak-kinds=all ./test_pool - ``` +These are explicitly out of the current build and tracked as issues: - - **Success criterion:** `ERROR SUMMARY: 0 errors from 0 contexts`. -3. **Performance Benchmark:** Compare the execution times of `memory_pool_alloc`/`free` against standard `malloc`/`free` over a loop of 1,000,000 iterations. +- **`std::pmr::memory_resource` adapter** — the "door left open" in [ADR-0018](../adr/0018-stl-allocator-adapter.md) (issue #107). +- **Coverage-guided fuzzing harness** (issue #108). +- **Opt-in debug hardening** — freed-block poisoning, canaries, free-list safe-linking; would also add double-free detection (issue #109). +- **Benchmark extension** — external baselines (jemalloc/tcmalloc) and p99 percentile reporting. +- **C4 component diagram** of the pool internals.