Repository navigation
refactor(writing)!: remove GlobalHeapFlushThreshold - #197
Open
Blackclaws wants to merge 32 commits into
Open
Blackclaws wants to merge 32 commits into
Blackclaws wants to merge 32 commits into
Conversation
Each header was written to the file and read back to compute its checksum: an async read per object header, flushing the stream's write buffer every time. It is now encoded into a pooled buffer, checksummed there and written once. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ading back The superblock, the fixed-array index header and data block were written, read back and checksummed from what was read. A chunked dataset's object header was patched in the file and its checksum recomputed the same way. All of them now encode into memory and checksum there; a chunked dataset keeps a copy of its header to patch. Writing no longer reads from its destination at all, which a forward-only upload holding only the part it is filling needs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Structure is allocated in encode order, so it ends up spread evenly through the
file. A reader that fetches ranges rather than seeking freely therefore pays for
the whole file in order to read the structure: on a 620 MB reporting file,
walking the 8.2% of bytes that are structure touches 591 of 591 one-megabyte
ranges.
H5WriteOptions.MetadataPlacement adds two alternatives to the encode-order
layout, which stays the default:
Interleaved in encode order. The default, and what the writer has always
produced, byte for byte.
Aggregated structure is allocated from large blocks, so it forms a few
clusters. Needs no estimate of the total.
FrontLoaded structure is allocated from one region reserved at the front, so
a reader can fetch all of it in one range.
Measured on 2,000 groups, each with a contiguous dataset and a string attribute,
at 1 MiB ranges:
interleaved 16,878,158 bytes structure in 17/17 ranges
front-loaded 16,890,494 bytes structure in 1/17 ranges +0.07%
The reservation is sized by measurement rather than by estimate: a pass encodes
the file against a stream that discards everything and reads the metadata total
off the allocator. That is exact because it is the same encoder and the same
allocator, which is what lets it account for the two things an estimate cannot -
a chunk index's size scales with chunk count, and global heap collections are
allocated at a 4 kB minimum each and left partly empty. Set MetadataReservation
to skip the pass.
The pass does not compress. No metadata size depends on compressed output - a
filtered chunk index entry's size field is sized from ChunkByteSize, the
UNCOMPRESSED chunk size - and compression is around 97% of a filtered write, so
the pass costs at most 1.6% of one. On an unfiltered write the share is larger,
but such a write is cheap to begin with: 264 MB in 324 ms.
The writer allocates every address before writing and then seeks to it, so
segregating regions needs no change to any encoding logic - only the allocator,
plus classifying the allocation sites as structure or payload. Two of those
sites are not what they look like:
- An implicit chunk index is not an index at all. The chunks sit contiguously
from the address in the layout message and a chunk's address is arithmetic on
it, so what is allocated there is the whole payload of an unfiltered chunked
dataset. Counted as structure it would front-load the payload along with
everything else and dissolve the separation, silently - the file stays correct
and the same size.
- A compact dataset's payload lives inside its object header and makes no
allocation of its own, so for datasets under 64 kB payload IS structure and
there is nothing to separate. Files that lean on PreferCompactDatasetLayout
will see little from this.
Two more things worth knowing:
- There is no free list, so a region's unused remainder is abandoned rather than
reclaimed; that is the file-size cost above. A reservation that comes up short
spills into blocks rather than failing.
- The end-of-file address must cover allocated rather than written space, because an
address can be referenced without ever being written to: a dataset created through
BeginWrite has its payload allocated and named by its layout message whether or not
the caller ever writes it. Nothing extends the stream over that payload, and the
placement decides whether anything else does - an interleaved file usually has
metadata allocated after it, whose writes extend the stream by accident, while a
front-loaded one serves metadata from the front and ends before the payload the
layout points at. h5dump then rejects the dataset: "invalid dataset size, likely
file corruption". So the stream is extended to the allocator's mark rather than the
address being trimmed to the stream - trimming would be smaller, since an abandoned
region tail is usually the last thing in the file and referenced by nothing, but it
cannot be told apart from the payload case and getting it wrong truncates data.
Writing variable-length data through BeginWrite is the one case a measurement
cannot reach: how much global heap such data needs follows from the values, which
the pass has not seen. Everything else about a deferred dataset is covered, since
a chunk index is sized from the chunk count and that comes from the dimensions.
Set MetadataReservation there.
Not a breaking change: nothing is removed, renamed or altered in signature,
H5WriteOptions gains properties, the placement enum is new, and the default
places exactly as before.
Asserts the locality of the placement from the reader's side rather than from the written bytes: a stream that serves reads in fixed-size blocks and counts them stands in for an HTTP range-request client, and the same tree is written both ways so the only variable is where the structure went. Front loading has to at least halve the transfer and hold fewer blocks. At a 256 kB block over 600 groups, each with a deflated dataset and two attributes: interleaved 3,666,125 B fetched of 3,666,125 B 100.0% of file 14 requests front-loaded 262,144 B fetched of 3,678,461 B 7.1% of file 1 request Deflate is not incidental - without a filter these datasets are stored compact, payload inside the object header, and there is no separation left to measure. Co-Authored-By: Claude <noreply@anthropic.com>
The pass exists to total up metadata, so it only has to do work that decides how
much metadata there is. Copying fixed-size payload into a layout decides nothing:
a contiguous region and an implicit chunk index are raw data, a fixed-array index
is sized from the chunk count, and every index's encoded width comes from the
dataspace rather than from a value. So for fixed-size elements the pass now skips
the data path entirely - no selection walk, no chunk buffers, no byte copying, no
raw chunk allocation - on top of already skipping compression.
The pass still encodes data whose encoding allocates:
compact payload lives inside the object header, so here data IS
metadata
strings, jagged variable-length data takes global heap space sized from the
arrays, structs values themselves
holding either
object refs encoding one encodes the object it points at, header and all
Nullable<T> encoded as a variable-length sequence of one, so it takes
global heap space too - and holds no reference, so the
reference test does not see it
The test is on the element type rather than on the dataset type, because an array
is itself a reference: asked of the dataset type it answers yes for every
array-backed dataset and nothing would be skipped.
Sizing pass against a full write of the same 64 MB file, best of seven runs each -
the fastest run being the one least perturbed by whatever else the machine is doing:
unfiltered 34.9 ms -> 1.3 ms 43% of a write -> 2%
filtered 29.4 ms -> 0.9 ms 2.6% of a write -> 0.1%
MetadataLayoutTests asserts per shape that the pass still measures exactly what a
write allocates, which is what the shortcut rests on - an over-count wastes the
reservation's tail and an under-count spills.
A block is claimed in full, so a fixed block size is a floor on file size rather
than a proportional cost. At the 8 MB default, 33 kB of metadata produced an
8.4 MB file - 205 times the interleaved layout of the same content. Aggregation
paid it on any file whose metadata was under 8 MB, and a front-loaded write paid
it too whenever its reservation was exhausted, which deferred variable-length
data guarantees: the global heap such data needs is sized from values the sizing
pass never saw.
Blocks now start at 64 kB and double up to MetadataBlockSize, which bounds what
is claimed at roughly twice what is used. Deferred strings, 33 kB of metadata:
interleaved 40,974 bytes
aggregated 73,584 bytes was 8,396,656
front-loaded 86,078 bytes was 8,409,150
The cost is a handful of clusters rather than one, since the ramp adds at most
log2(MetadataBlockSize / 64 kB) blocks before reaching the ceiling - a constant,
after which the steady state is what it was. Measured over the range-fetching
stream from RangeReadCostTests, 3,000 groups with deflated datasets at 256 kB
blocks:
file fetched requests
interleaved 18,343,706 18,343,706 70
aggregated 18,378,700 1,835,008 7 was 25,784,268 / 4
front-loaded 18,356,042 1,048,576 4
So aggregation trades three range requests for not inflating the file by 41%, and
front loading - the placement to reach for when round trips matter - is untouched.
MetadataBlockSize becomes a ceiling rather than a size; nothing about it changes
for a file with enough metadata to reach it.
MetadataAbandoned counted a region's remainder only when the region was replaced. A region is replaced only when a request does not fit it, so those remainders are small by construction, while the tail of the region left open at the end - the part that is actually large - was never counted at all. It reported 48 bytes of waste on a write that had claimed 8 MB and used 33 kB of it. It now includes the open region's tail, which makes it agree exactly with what the file grew by. Those are independently measurable: growth is observed from outside, abandonment reported from inside, and since a region is claimed in full and there is no free list, the two are the same number. AbandonedSpaceAccountsForTheFileGrowth asserts that equality rather than a bound. The figure is live rather than final while a write is in progress - it answers what would be abandoned if the write ended now. Nothing in the library reads it; it is diagnostic.
The comparison covered interleaving against front loading only, which leaves the
choice between the two clustered placements unmeasured. They answer different
questions - a reservation is sized by measuring the file first and gets the
structure into one range, aggregation needs no such pass and settles for a handful
- so a caller picking between them wants the gap, and a caller who cannot afford
the sizing pass wants to know aggregation is still worth having.
600 groups, deflated datasets, 256 kB blocks, on .NET 8:
file fetched requests
interleaved 3,666,125 3,666,125 14
aggregated 3,673,087 524,288 2
front-loaded 3,678,461 262,144 1
Aggregation must halve what a walk transfers, and a measured reservation must not
do worse than not measuring. It must also not buy locality with file size: blocks
are claimed in full, so a block of fixed size is a floor rather than a proportional
cost, and at the 8 MB default this file came out three times larger while still
walking in one request. That is bounded here from the reader's side, where the
trade is visible, as well as in MetadataLayoutTests from the writer's.
The payload was a strided integer ramp, chosen only so that it would not compress
away to nothing. That turns out to be pathological for a fast match finder: .NET 9
replaced the bundled zlib with zlib-ng, whose match finder differs below maximum
effort, and this shape compressed 73% worse there than on .NET 8. Every figure the
test reports moved with it, which made the numbers quoted for the placements a
property of the target framework as much as of the layout.
It is now a periodic signal with a slow drift, rounded to two decimals - which is
what these files actually hold. It compresses to about 39%, so the file is neither
dominated by payload nor too small for locality to be expressible, and it lands
within 0.2% on both runtimes:
.NET 8 .NET 10
interleaved 4,050,543 4,058,242
aggregated 4,052,705 4,060,404
front-loaded 4,062,879 4,070,578
Fetched bytes and request counts are now identical on both: 16 requests and the
whole file interleaved, 3 and 786,432 B aggregated, 1 and 262,144 B front-loaded.
The rounding is what buys the compression - full-mantissa doubles differ in their
low bytes at every sample and barely compress - and 2,048 of them is the same 16 kB
per group as before, so the file stays the size the block arithmetic assumes.
The guidance for deferred writes is that MetadataReservation is worth setting for variable-length data and unnecessary for fixed-size data. That rested on reading the code rather than on a measurement. filtered chunked int (fixed array) measured 495 allocated 495 shortfall 0 unfiltered chunked int (implicit index) measured 192 allocated 192 shortfall 0 contiguous strings (global heap) measured 200 allocated 295,112 shortfall 294,912 A chunk index is fully accounted for however the chunks are indexed, because its size follows from the chunk count and that follows from the dimensions, which are fixed when the dataset is declared: WriteChunkInfos is sized to TotalChunkCount in H5D_Chunk4.Initialize, and writing data later fills entries in an array whose length was settled then. Global heap space is not accounted for, because how much a variable-length value needs follows from the value. So the shortfall is zero for one and 288 kB for the other, which is the distinction the guidance draws, now asserted rather than reasoned.
The allocator counts addresses from the superblock; the driver's Length and SetLength count from the start of the file. A user block puts those a UserBlockSize apart, and the extension compared them untranslated - so with a user block the file looked long enough already and the extension was skipped. That matters for exactly the case the extension exists for: a dataset declared through BeginWrite and never written has its payload allocated and named by its layout message, and nothing else extends the stream over it under a clustered placement. h5dump rejects the result: H5D__contig_check(): invalid dataset size, likely file corruption for both Aggregated and FrontLoaded at UserBlockSize = 512, where the same file with no user block is clean. Interleaved survived by accident, as it does without a user block - metadata allocated after the payload extends the stream on its own. EndOfFileAddressTests now runs each placement with and without a user block. The zero case cannot stand in for the nonzero one, since the whole defect is the translation between the two coordinate systems. 512 rather than a larger user block because that is the only size this library round-trips today: the superblock search in NativeFile.InternalOpenAsync doubles its step while seeking relatively, so it probes 512, 1536, 3584 rather than the 512, 1024, 2048 the format specifies. That is a reader bug of its own, unrelated to placement, and is reported separately.
The reservation was the measured total plus 3 * 4096 bytes of slack, justified on the grounds that the measurement counts the superblock while the superblock is served from outside the region, "leaving the region short by exactly that much". It leaves the region LONG by that much. Subtracting rather than adding makes the reservation the exact number of bytes that will be served from it. Front loading therefore costs nothing at all rather than a little. Measured across five shapes - contiguous, filtered chunked, variable-length strings, Nullable<T>, 200 groups - the cost over the interleaved layout of the same content: measured + 3 * 4096 12,336 bytes abandoned measured 48 bytes abandoned measured - superblock 0 bytes abandoned One region opened in every case, at every setting: the slack was never keeping a final allocation from spilling, because nothing was spilling. On the 2,000-group fixture a front-loaded file is now 16,878,158 bytes against an interleaved 16,878,158, with structure in 1 of 17 ranges rather than 17. Nothing caught this. AutoSizedFrontLoadingCostsNothing now asserts the sizes are equal rather than within half a percent, and asserts MetadataRegionsOpened is 1 with nothing abandoned - the property that distinguishes a measurement that was short (spills, losing the locality) from one that was long (pays for a tail nothing uses). MetadataRegionsOpened existed to report exactly that and was checked nowhere. Also in this file, two claims that did not survive being measured: - MetadataLayoutTests.Options reserved 3 MB and said 1 MB would spill into a second region. The measured need is 878,158 bytes and 1 MB opens one region, so the reservation is now 1 MB and the comment says what it is for - exercising the path that skips the sizing pass, at the price an explicit reservation pays, which is its unused tail. That price was 13.44% of the file and is now 1.01%. - MeasureBaseline wrote with default options, so it ran with compact layout ON, which the class remark calls out as the one thing that must be off for these files to measure anything.
…a heap Global heap collections were all allocated as structure, justified on the grounds that the heap holds attribute payload - "what a viewer reads while browsing rather than while reading a dataset". That is true of an attribute's value and false of a dataset's own variable-length elements, which go through the same manager. A dataset of variable-length strings with no attributes at all measured 97.3% structure, so a front-loaded region swallowed the payload and there was nothing left to separate. Silently: the file stays valid and the same size, and only the locality quietly fails to materialise. The manager now keeps one collection open per allocation kind and allocates each with its own kind. Per kind rather than per object, because a collection is allocated whole - one shared collection would put both kinds at whichever address the first object claimed. vlen strings as dataset, no attributes 97.3% -> 0.0% jagged arrays as dataset, no attributes 97.3% -> 0.0% one large string as an attribute 100% -> 100% vlen dataset plus a string attribute 0.4% Which kind is being encoded cannot be passed in: the delegates that call AddObject come from DatatypeMessage.Create, shared by both paths. So the writer scopes it at the two points where it can - a dataset's data write and an attribute's encode - and both restore it, so nesting stays correctly attributed. An object reference among a dataset's elements encodes the object it points at, and that object's attributes are still attributes. Two consequences worth knowing: - A deferred write now needs no reservation at all. Variable-length data written after BeginWrite was the one case a measurement could not reach; its heap is payload now, so the shortfall is zero and the file comes out the size an interleaved one does. NothingDeferredOutrunsTheSizingPass asserts that. - GlobalHeapManager's flush now clears every open collection rather than one. A flush encodes and drops the whole map, so a collection left open afterwards would take objects nothing would ever encode again.
…rate options Three assertions that could not fail, and one option that meant nothing. DumpH5File strips h5dump's HDF5-DIAG error stack so the 134 fixture comparisons do not depend on the locally installed version. That also strips every trace of failure, so asserting the dump does not contain "error" always passed. On a file h5dump rejects outright - lzf.h5, exit code 1 with 2,910 bytes of error stack - the stripped output contains the word "error" zero times. RunH5Dump now reports the exit code and both streams unmodified, and the placement and end-of-file tests ask it whether the C library accepted the file. DumpH5File keeps its current output for the fixture comparisons. It also returns null when h5dump is not installed rather than throwing Win32Exception, which makes Skip.If(dump is null) do what it says. It was dead code: these tests failed rather than skipped on a machine without h5dump, which would have hit CI or a contributor and not me. MetadataBlockSize of zero or less silently turned Aggregated into Interleaved - the block path is gated on a positive size, so a caller asked for clustering, got none, and had nothing to tell them why. That and a negative MetadataReservation now throw. Interleaved still ignores the block size, because it genuinely uses no blocks. And RangeReadCostTests now asserts the request count. It is the headline figure of this feature and the unit every claim about it is stated in, and it was printed and never checked: front loading must take exactly one request, and aggregation a handful against interleaving's one per block.
…ttering it
Classifying a dataset's variable-length values as payload fixed what a reader
fetches and broke what a writer can stream, and both matter.
A global heap collection is allocated when it is needed and written much later,
when it flushes. A writer uploading to object storage buffers the file in fixed
parts and can only ship a part once every byte in it is written, so such a
collection is a hole that pins its part until the flush. Scattered through the
file, those holes pin every part and the whole object stays in memory. Gathered,
they pin a few.
Measured over that upload model - 2,000 groups, a fixed-size dataset and one small
variable-length value each, 1 MB parts, so payload dominates and the values pack
into shared collections:
variable-length values as ATTRIBUTES parts held shipped during the write
Interleaved 33 0
Aggregated 6 28
FrontLoaded 3 31
variable-length values as DATASETS
Interleaved 33 0
Aggregated was 7, 33 after the reclassification, now 9
FrontLoaded was 3, 33 after the reclassification, now 6
So the reclassification alone cost 3 parts to 33 - 3.1 MB resident against 34.6 MB,
scaling with file size rather than staying flat.
AllocationKind.DeferredRawData is therefore payload that is clustered: its own
blocks, never the metadata region, so a reader walking the structure still does not
fetch it while a writer still sees parts complete. Six rather than the original
three is the honest price of two clusters instead of one, which is what keeping
payload out of the structure region costs.
Chunk and contiguous payload needs none of this - it is written at the address it
was allocated and leaves no hole.
The clustering logic is now shared, since two independent streams of allocations
need it, and PayloadAbandoned reports the second region's unused tail.
MetadataAbandoned and MetadataRegionsOpened stay structure-only: a reservation
should not be sized against payload. Together the two figures still account for
every byte a clustered file carries that an interleaved one does not, which
AbandonedSpaceAccountsForTheFileGrowth asserts.
The data block's size follows from the chunk count, which is fixed when the layout is created, so it is allocated together with the index header instead of when the writer is disposed. All of the index is then structure that a front-loaded reservation covers at the time it is placed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Front loading measured the structure by encoding the whole tree against a discarding stream, then encoded it again for real, because every payload address depends on how large the structure turns out to be. The structure is now encoded once and held in memory until it is complete. Payload is placed provisionally beyond any real file meanwhile; once the structure's size is known, the payload moves down behind it, the headers that name a provisional address are corrected and re-checksummed, and the structure is written front to back in one run. Data given with the tree is written after that. A compact dataset's variable-length values are placed as structure in every placement now, since its data is part of its header. At 300,000 datasets BeginWrite takes 0.67 s instead of 1.01 s and allocates 427 MB instead of 687 MB. Peak memory rises by the size of the structure while it is held: 34 MB there. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A front-loaded write encodes its structure once and holds it until it is complete, so nothing measures the structure by encoding it beforehand any more. The pass, the discarding stream it wrote to, and the paths that let it skip data and compression go, along with the tests that compared its total with a write's. That front loading reserves exactly what a write allocates is asserted by SinglePassFrontLoadingTests, for the same shapes. The documentation of MetadataReservation and FrontLoaded now describes holding the structure in memory instead of measuring it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A count for every object cost one dictionary entry per dataset for the writer's whole lifetime, to record a number that is almost always one. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Only a chunked layout carries state from encoding to Dispose. Holding the others for the writer's lifetime cost a layout object and a cached stream per dataset, the largest per-dataset cost for files of many small datasets. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
MakeGenericMethod plus Invoke per dataset allocated a method, its type arguments and an argument array for every dataset encoded and every write. Exceptions thrown while writing data now surface unwrapped instead of inside a TargetInvocationException, as they already did for encoding. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
IsReferenceOrContainsReferences and the unmanaged-memory datatype lookup each built a closed generic method per call, which encoding asks for once per dataset. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…asets Their dataset info, layout and dataspace messages are rebuilt per write instead of held until Dispose. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tions When the data covers a contiguous dataset exactly and no selection narrows it, the general path ends in a single encoder call anyway. Making that call directly skips the selection walk and the objects built for it. Anything else still takes the general path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The address map and the dataset map were keyed by the same objects. One map of addresses with the deferred state of a dataset alongside, held through a single reference, replaces both. Group links and object references now share one encode-once path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A dataset carried a field for each of its chunks, selections, creation properties, opaque info and null dataspace flag, which most datasets never set, an array for one-dimensional dimensions, and its type, which its data or its type argument already says. 300,000 datasets declared by dimensions took 41 MB for that. Options a dataset is given are kept in one object it holds only if given any, a single dimension is kept inline, and the type is derived. A tree of 300,000 datasets in groups of five takes 54 MB instead of 86 MB. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A group kept its links in a dictionary, about 380 bytes for a group of five, which made the groups' tables a third of a tree of many small groups. Up to eight links are kept in an array of pairs now, searched in order, and moved into a dictionary once there are more. A tree of 300,000 datasets in groups of five takes 43 MB instead of 54 MB. BREAKING CHANGE: while a group holds at most eight links, its Keys and Values are copies taken when they are read, not views that follow later changes. Links keep the order they were added in after one is removed, where a dictionary may reuse the removed link's place. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…f their own A dataset write's variable-length values went into minimum-sized collections shared with everything else, written whenever the heap was next flushed. A write whose values fill at least one collection is now measured first, by encoding it once against a discarding target, and its values go into adjacent collections sized exactly for them (split at 1 MiB, since a reader loads a collection whole, and at the object limit), written as the write ends. Smaller writes keep sharing collections. A value too large for a standard collection gets one of its own instead of replacing the open collection and abandoning its free space. Collections are now written only between top-level encodes: at the end of BeginWrite, after each Write once over GlobalHeapFlushThreshold, and at Dispose. Flushing when a new collection was opened could write a nested variable-length value's collection while that value was still being filled, losing the rest of it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A shared collection that could take no more objects stayed unwritten until 4 MB of them had accumulated or the writer was disposed, each one a hole that an upload in parts has to hold its part open for. They are now written at the end of every Write, leaving at most the collection still filling per kind. GlobalHeapFlushThreshold is no longer used; it is kept so that code setting it still compiles. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Full heap collections are written at the end of every write, so the threshold no longer controls anything. BREAKING CHANGE: H5WriteOptions no longer has a GlobalHeapFlushThreshold parameter. Code setting it by name no longer compiles; drop the argument. Code passing H5WriteOptions arguments positionally past MinimumGlobalHeapCollectionSize has to drop the third one. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Arkane5
reviewed
Nov 5, 2026
| if (_pendingCollections.Sum(state => (long)state.Collection.CollectionSize) >= _options.GlobalHeapFlushThreshold) | ||
| Encode(); | ||
| var open = _openCollections.Values; | ||
| var closed = _pendingCollections.Where(state => !open.Contains(state)).ToList(); |
Contributor
There was a problem hiding this comment.
Since _pendingCollection is a List<T>, you could use :
var closed = _pendingCollections.FindAll(state => !open.Contains(state));
This branch has not been deployed
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
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.
Builds on #196, after which the option controls nothing: full heap collections are written at the end of every write.
Split out so that the breaking part can be declined on its own. Nothing else in the stack depends on it.
Breaking change
H5WriteOptionsno longer has aGlobalHeapFlushThresholdparameter. Code setting it by name no longer compiles; drop the argument. Code passingH5WriteOptionsarguments positionally pastMinimumGlobalHeapCollectionSizehas to drop the third one.🤖 Generated with Claude Code