Rename maybeDecompressedStringAsNumber and propagate mayContainCompressedIds to non-leaf nodes#26980
Merged
CraigMacomber merged 5 commits intomicrosoft:mainfrom Apr 10, 2026
Conversation
…ds to non leaf nodes as well
Contributor
There was a problem hiding this comment.
Pull request overview
This PR clarifies and refines how UniformChunks represent and interpret ID-compressed values by renaming the leaf opt-in flag, introducing a derived mayContainCompressedIds signal for entire shapes, and using that signal to avoid retaining an idCompressor reference when it cannot be needed.
Changes:
- Replaced the old leaf-only flag with a clearer leaf opt-in (
maybeCompressedIdLeaf) and a derived shape-wide indicator (mayContainCompressedIds) that propagates from descendants. - Updated
UniformChunkto only retainidCompressorwhen the chunk’s shape indicates compressed IDs may be present. - Updated/added tests to validate invalid flag usage and propagation of
mayContainCompressedIds.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/dds/tree/src/feature-libraries/chunked-forest/uniformChunk.ts | Adds mayContainCompressedIds, derives it from child shapes, updates cursor decompression logic, and conditionally retains idCompressor. |
| packages/dds/tree/src/feature-libraries/chunked-forest/chunkTree.ts | Updates JSDoc around when UniformChunk compression applies. |
| packages/dds/tree/src/test/feature-libraries/chunked-forest/uniformChunk.spec.ts | Switches to assertion-based failure validation and adds propagation tests for mayContainCompressedIds. |
Josmithr
reviewed
Apr 9, 2026
Josmithr
reviewed
Apr 9, 2026
Josmithr
reviewed
Apr 9, 2026
Josmithr
reviewed
Apr 9, 2026
Josmithr
reviewed
Apr 9, 2026
Josmithr
reviewed
Apr 9, 2026
Josmithr
reviewed
Apr 9, 2026
CraigMacomber
commented
Apr 9, 2026
| // Slow path: walk shape and cursor together, inserting values. | ||
| if (shape.hasValue) { | ||
| if ( | ||
| shape.mayContainCompressedIds && |
Contributor
Author
There was a problem hiding this comment.
this "fix" is not as important as it might seem, since we do not support unions in uniform chunks currently, so this could never put a compressed id where a number is valid. Therefor there could have been no impacted users. With this change however, where we discard the id compressor, this change is required to ensure decode always works.
Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com>
Josmithr
reviewed
Apr 9, 2026
Josmithr
reviewed
Apr 9, 2026
…Chunk.ts Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com>
Josmithr
approved these changes
Apr 9, 2026
agarwal-navin
pushed a commit
to agarwal-navin/FluidFramework
that referenced
this pull request
Apr 13, 2026
…ssedIds to non-leaf nodes (microsoft#26980) ## Description Improves clarity around how compressed ids are handled in UniformChunks, and tracks if a chunk might use them (deeply) in the shape. As a memory optimization, avoid storing idCompressor when unneeded. For small leaf chunks (as is common currently) this should help memory use a little as there are a very large number of such chunks.
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.
Description
Improves clarity around how compressed ids are handled in UniformChunks, and tracks if a chunk might use them (deeply) in the shape.
As a memory optimization, avoid storing idCompressor when unneeded. For small leaf chunks (as is common currently) this should help memory use a little as there are a very large number of such chunks.
Reviewer Guidance
The review process is outlined on this wiki page.