-
Notifications
You must be signed in to change notification settings - Fork 557
Container: Enable getPendingLocalState by default #25631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Container: Enable getPendingLocalState by default #25631
Conversation
And flip the setting on the empty batch test
…to offline-config-cleanup
…to offline-config-cleanup
…to offline-config-cleanup
…to offline-unify-on-isnapshot
…to offline-unify-on-isnapshot
…to attach-isnapshot
…to offline-unify-on-isnapshot
…to offline-unify-on-isnapshot
…to offline-unify-on-isnapshot
…to offline-unify-on-isnapshot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables the getPendingLocalState
functionality by default for containers, while optimizing the memory overhead by switching the in-memory snapshot representation from SnapshotWithBlobs
to ISnapshot
. The change eliminates the need for the "Fluid.Container.enableOfflineLoad"
configuration flag across test files.
Key changes:
- Default enables offline load capability (
enableOfflineLoad !== false
instead of explicit opt-in) - Optimizes snapshot memory usage by using
ISnapshot
format internally and deferring conversion toSnapshotWithBlobs
until needed - Removes test configuration dependencies on the now-unnecessary feature flag
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
packages/loader/container-loader/src/serializedStateManager.ts | Core implementation changes for snapshot handling and default offline load enabling |
packages/loader/container-loader/src/container.ts | Updates container initialization to enable offline load by default |
packages/loader/container-loader/src/utils.ts | Updates utility functions for snapshot conversion |
packages/loader/container-loader/src/containerStorageAdapter.ts | Updates type references and snapshot conversion calls |
Multiple test files | Removes "Fluid.Container.enableOfflineLoad": true configuration from test setups |
packages/loader/container-loader/src/test/snapshotConversionTest.spec.ts
Outdated
Show resolved
Hide resolved
…st.spec.ts Co-authored-by: Copilot <[email protected]>
…ny-murphy/FluidFramework-1 into offline-unify-on-isnapshot
The PR enables calling getPendingLocalState by default, and ensures the overhead of capturing the pending state is minimized and/or deferred to the getPendingLocalState call itself. The primary means by which we reduce the overhead is my switching the in-memory representation of the snapshot from SnapshotWithBlobs to ISnapshot.
ISnapshot has a number of advantages as an in-memory format, primarily it matches the native format returned by modern drivers, and includes both the snapshot tree, as well as the snapshot blobs as ArrayBuffers. Since this format matches the driver's format, which eventually gets passed down into the runtime, we ensure we are not storing a duplicate copy, which could have significant memory overhead for large documents. SnapshotWithBlobs on the other had stores blobs as strings, which is great for serialization, but is redundant with the ArrayBuffers the rest of fluid operates over. In getPendingLocalState we still need to convert to SnapshotWithBlobs as the result is the serializable, but this is deferred until it is needed.
For compatibly with older drivers, we still maintain support for driver that do not return ISnapshot, but now we defer blob retrieval until getPendingLocalState is called. This ensures the load path doesn't need to retrieve any additional data than what is required by the load flow.
Outside of the container-loader package, all updates are just to test to remove the old config, as it is no longer necessary to explicitly enable the feature.