Skip to content

Commit 6d27e86

Browse files
anthony-murphymarkfieldsCopilot
authored
Container: Enable getPendingLocalState by default (#25631)
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. --------- Co-authored-by: Mark Fields <markfields@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 72648b6 commit 6d27e86

23 files changed

+229
-322
lines changed

experimental/dds/tree/src/test/utilities/TestUtilities.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,6 @@ export async function setUpLocalServerTestSharedTree(
321321
} = options;
322322

323323
const featureGates = options.featureGates ?? {};
324-
featureGates['Fluid.Container.enableOfflineLoad'] = true;
325324

326325
const treeId = id ?? 'test';
327326
let factory: SharedTreeFactory;

packages/dds/tree/src/test/utils.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ import {
3838
} from "@fluidframework/test-runtime-utils/internal";
3939
import {
4040
type ChannelFactoryRegistry,
41-
type ITestContainerConfig,
4241
type ITestObjectProvider,
4342
type SummaryInfo,
4443
TestContainerRuntimeFactory,
@@ -351,16 +350,9 @@ export class TestTreeProvider {
351350
const configProvider = (settings: Record<string, ConfigTypes>): IConfigProviderBase => ({
352351
getRawConfig: (name: string): ConfigTypes => settings[name],
353352
});
354-
const testContainerConfig: ITestContainerConfig = {
355-
loaderProps: {
356-
configProvider: configProvider({
357-
"Fluid.Container.enableOfflineLoad": true,
358-
}),
359-
},
360-
};
361353
const container =
362354
this.trees.length === 0
363-
? await this.provider.makeTestContainer(testContainerConfig)
355+
? await this.provider.makeTestContainer()
364356
: await this.provider.loadTestContainer();
365357

366358
this._containers.push(container);

packages/loader/container-loader/src/container.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,7 +1023,7 @@ export class Container
10231023
this.isInteractiveClient &&
10241024
(this.mc.config.getBoolean("Fluid.Container.enableOfflineLoad") ??
10251025
this.mc.config.getBoolean("Fluid.Container.enableOfflineFull") ??
1026-
options.enableOfflineLoad === true);
1026+
options.enableOfflineLoad !== false);
10271027
this.serializedStateManager = new SerializedStateManager(
10281028
this.subLogger,
10291029
this.storageAdapter,
@@ -1668,8 +1668,11 @@ export class Container
16681668
timings.phase2 = performanceNow();
16691669

16701670
// Fetch specified snapshot.
1671-
const { baseSnapshot, version, attributes } =
1672-
await this.serializedStateManager.fetchSnapshot(specifiedVersion, pendingLocalState);
1671+
const {
1672+
snapshot: baseSnapshot,
1673+
version,
1674+
attributes,
1675+
} = await this.serializedStateManager.fetchSnapshot(specifiedVersion, pendingLocalState);
16731676
const baseSnapshotTree: ISnapshotTree | undefined = getSnapshotTree(baseSnapshot);
16741677
this._loadedFromVersion = version;
16751678

packages/loader/container-loader/src/containerStorageAdapter.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { ProtocolTreeStorageService } from "./protocolTreeDocumentStorageService
3131
import { RetriableDocumentStorageService } from "./retriableDocumentStorageService.js";
3232
import type {
3333
ISerializedStateManagerDocumentStorageService,
34-
ISnapshotInfo,
34+
SerializedSnapshotInfo,
3535
} from "./serializedStateManager.js";
3636
import { convertSnapshotInfoToSnapshot } from "./utils.js";
3737

@@ -88,7 +88,9 @@ export class ContainerStorageAdapter
8888
* ArrayBufferLikes or utf8 encoded strings, containing blobs from a snapshot
8989
*/
9090
private readonly blobContents: { [id: string]: ArrayBufferLike | string } = {},
91-
private loadingGroupIdSnapshotsFromPendingState: Record<string, ISnapshotInfo> | undefined,
91+
private loadingGroupIdSnapshotsFromPendingState:
92+
| Record<string, SerializedSnapshotInfo>
93+
| undefined,
9294
private readonly addProtocolSummaryIfMissing: (summaryTree: ISummaryTree) => ISummaryTree,
9395
private readonly enableSummarizeProtocolTree: boolean | undefined,
9496
) {
@@ -184,10 +186,7 @@ export class ContainerStorageAdapter
184186
const localSnapshot =
185187
this.loadingGroupIdSnapshotsFromPendingState[snapshotFetchOptions.loadingGroupIds[0]];
186188
assert(localSnapshot !== undefined, 0x970 /* Local snapshot must be present */);
187-
snapshot = convertSnapshotInfoToSnapshot(
188-
localSnapshot,
189-
localSnapshot.snapshotSequenceNumber,
190-
);
189+
snapshot = convertSnapshotInfoToSnapshot(localSnapshot);
191190
} else {
192191
if (this._storageService.getSnapshot === undefined) {
193192
throw new UsageError(

0 commit comments

Comments
 (0)