Dzint/resize connectivity vectors - #1056
Merged
Merged
Conversation
TriMesh and TetMesh each carried two copies of the same bump allocator -- request_tri_slots/request_tet_slots and their respective request_vert_slots. After the hash alignment in 340d429 those four functions differed only in which atomic counter and which storage vector they named: same CAS loop, same refuse-rather-than-resize, same reset of the slots handed out. SlotPool<T> holds the storage and its counter together. The invariant that matters is live() <= capacity(), and it can only be maintained where both are in reach -- as separate members there was nothing to stop a counter being paired with the wrong vector. It is also the single place a future growth strategy has to be implemented, rather than four. The two numbers are now named apart. tri_capacity()/tet_capacity() return the LIVE count rather than the storage size, which is what made the spare region hard to reason about; SlotPool calls them live() and capacity(). The entry points behave identically now. request_*_slots returns size_t and reports refusal as INVALID_SLOT instead of -1, and each get_next_empty_slot_* is just request_*_slots(1). TetMesh's wrappers previously returned int, so callers there tested `< 0` while TriMesh's tested `== size_t(-1)`; unifying them also drops an (int) narrowing that silently capped tet indices at 2^31. The capacity check is phrased as `n > cap - first` rather than `first + n > cap`, which cannot wrap now that the arithmetic is unsigned. Testing that rewrite exposed a gap. An off-by-one at the capacity boundary was NOT caught by the existing tests: every assertion was framed against cell_capacity(), the live count, so letting the live count run one slot past the storage stayed internally consistent. live() <= capacity() simply was not observable through the meshes. SlotPool being standalone makes it directly testable, and the added test case fails in three places under that mutation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A pass that exhausted the preallocated storage abandoned the affected
operations silently. Nothing detected it, and the capacity was only
re-derived at the end of the outer iteration -- from the same
m_preallocation_factor that had just proved insufficient, so a mesh that
needed more room kept losing the same work every iteration.
SlotPool now records that a request was refused. A flag rather than a
count: the response is to consolidate, which re-derives the capacity from
the real element count, so the size of the shortfall is not an input.
local_operations checks it at each op-group boundary and consolidates.
consolidate_mesh already does both halves of the job:
- it reclaims the slots removed elements still hold. The counter only
advances during a pass, so live() is an allocation high-water mark and
churn alone can exhaust the headroom without the mesh growing at all.
- it resizes to reserved_capacity(real count), which when there is no
churn to reclaim grows the storage by m_preallocation_factor.
The group is then retried in place, with the retry depth counted and
logged. Consolidate renumbers, so the failed operations' tuples are gone;
re-running the group re-collects them against the enlarged storage.
Also adds tri/tet/vert_storage_capacity(). *_capacity() reports the LIVE
count, so the storage size was not observable from outside the mesh --
which is what made this behavior undiagnosable, and what made an earlier
reading of it wrong.
simwild_spec.json gains /preallocation_factor. set_preallocation_factor_from_json
already read it, but the spec rejected it, so the knob was dead for simwild.
(It is still dead for the other components that read it without declaring it:
topological_offset, isotropic_remeshing, qslim, manifold_extraction,
shortest_edge_collapse.)
Measured on 441722.stl at preallocation_factor 1.05, where exhaustion is
guaranteed:
split pass exhausted: 113630 of 171845 tets reclaimed as churn,
storage 173887 -> 61126
split pass exhausted: 0 of 60997 tets reclaimed as churn,
storage 61126 -> 64047
split pass exhausted: 0 of 63923 tets reclaimed as churn,
storage 64047 -> 67120
The first fire is pure churn -- two thirds of the allocation reclaimed, and
the storage shrinks because the real mesh is far smaller than the high-water
mark. Later fires reclaim nothing and grow by the factor instead.
15 fires, max retry depth 4, converged in 21.3s. The same configuration ran
past 10 minutes without finishing before the retry existed. At the default
factor 6.0, nothing fires, and behavior is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.