Tedhar/misc fixes#1902
Merged
Merged
Conversation
Add a custom IOrderer (NamespaceTypeOrderer) that lays out the BDN summary table by namespace, then type, preserving normal within-type ordering (category, params, job, declared method). This keeps every namespace's results in one unbroken series when --join is used, instead of the default orderer interleaving the n-th declared method of every type. Wire the orderer and a visible Categories column into both the Release (BaseConfig) and Debug (DebugInProcessConfig) configs, and add [BenchmarkCategory] attributes to the three RawStringOperations classes (Network, Operations, Operations.LTM): Set/SetEx => Upsert, SetNx/SetXx and all *crement* methods => RMW, Get* => Read. Category names are centralized in BenchmarkCategories. Co-authored-by: Copilot <[email protected]>
…ation) Remove unused AddressInfo
…ame RDH.KeyLengthValueMask to .KeyLengthLowBitsMask for clarity (same for Value)
…ests to a more fitting sub-project
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes a set of targeted Tsavorite internal correctness/cleanup changes (notably around record metadata sizing and inline-size constraints), and improves BenchmarkDotNet output organization for Garnet’s BDN benchmarks. It also expands Tsavorite’s record-ops test coverage for inline/overflow/object shapes and ObjectIdMap concurrency/stress behaviors.
Changes:
- Improved Tsavorite record metadata handling and disposal ordering; allowed
MaxInline*Size == 0to support “pure object store” configurations. - Added/expanded Tsavorite record-ops tests (inline vs overflow vs object values, CopyToTail behaviors, ObjectIdMap stress/OOM-injection paths).
- Improved BDN benchmark reporting by adding benchmark categories and a namespace/type orderer (plus category column).
PR Metadata Notes
- Title:
"Tedhar/misc fixes"is underspecified for future history search; consider a scoped title like[Tsavorite] Allow zero inline sizes; fix record field info; expand record-ops tests(or similar). - Description: Generally detailed and maps well to the diff; it would benefit from an Issues Fixed link (if applicable) and a brief Tests section (commands run).
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| libs/storage/Tsavorite/cs/test/test.recordops/ObjectTests.cs | Adds coverage for MaxInlineValueSize == 0 behavior (overflow-only values). |
| libs/storage/Tsavorite/cs/test/test.recordops/ObjectInlineTests.cs | New tests for inline/overflow/object value conversions via Upsert/RMW paths. |
| libs/storage/Tsavorite/cs/test/test.recordops/ObjectIdMapTests.cs | New stress tests for ObjectIdMap/MultiLevelPageArray behaviors (incl. DEBUG OOM-injection path). |
| libs/storage/Tsavorite/cs/test/test.recordops/ModifiedBitTests.cs | Renames a test to clarify intent. |
| libs/storage/Tsavorite/cs/test/test.recordops/CopyToTailTests.cs | New tests for CopyToTail / CopyToReadCache across key/value shapes (incl. object values). |
| libs/storage/Tsavorite/cs/test/SimpleTests.cs | Removes a simple AddressInfo-focused test (paired with AddressInfo removal). |
| libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/TsavoriteBase.cs | Adjusts table initialization state updates and disposal cleanup (removes unused Free(version), disposes resize allocator). |
| libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/Tsavorite.cs | Reorders Dispose() so Free() runs after contained objects are disposed. |
| libs/storage/Tsavorite/cs/src/core/Index/Recovery/IndexRecovery.cs | Updates recovery path to rely on Initialize() rather than explicit version Free. |
| libs/storage/Tsavorite/cs/src/core/Index/Common/LogSettings.cs | Lowers MinMaxInlineSize to 0 to allow zero inline sizes. |
| libs/storage/Tsavorite/cs/src/core/Index/Common/AddressInfo.cs | Removes AddressInfo implementation (paired with test removal). |
| libs/storage/Tsavorite/cs/src/core/Allocator/RecordDataHeader.cs | Renames key/value length masks for clarity and updates call sites. |
| libs/storage/Tsavorite/cs/src/core/Allocator/LogRecord.cs | Fixes GetRecordFieldInfo() to report correct overflow sizes/record type/extended namespace length; typo fix. |
| benchmark/BDN.benchmark/Program.cs | Integrates the new orderer + categories column into debug/release configs. |
| benchmark/BDN.benchmark/Operations/RawStringOperations.cs | Adds benchmark categories for clearer grouping. |
| benchmark/BDN.benchmark/Operations/LTM/RawStringOperations.cs | Adds benchmark categories for clearer grouping. |
| benchmark/BDN.benchmark/Network/RawStringOperations.cs | Adds benchmark categories for clearer grouping. |
| benchmark/BDN.benchmark/NamespaceTypeOrderer.cs | New custom summary orderer to group results by namespace/type for joined summaries. |
| benchmark/BDN.benchmark/BenchmarkCategories.cs | New shared category constants for benchmark grouping. |
badrishc
approved these changes
Jun 26, 2026
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.
This makes several minor fixes to the Tsavorite storage engine internals.
Improvements to Garnet BDN's RawStringOperations output:
BenchmarkCategoriesclass to define and document benchmark categories (Upsert,RMW,Read), and applied these categories to relevant benchmark methods inRawStringOperationsand its variants for clearer grouping and analysis. [1] [2] [3] [4].NamespaceTypeOrdererto order benchmark results by namespace and type, improving the readability of joined summaries. Integrated this orderer and the category column into both debug and release benchmark configurations. [1] [2] [3]Tsavorite storage engine corrections and enhancements:
KeyLengthandValueLengthinRecordDataHeader, replacingkKeyLengthValueMask/kValueLengthValueMaskwithkKeyLengthLowBitsMask/kValueLengthLowBitsMask. [1] [2] [3] [4] [5] [6] [7] [8]LogRecord.GetRecordFieldInfo()to correctly report overflow key/value sizes, record type, and extended namespace length