Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions packages/dds/tree/src/shared-tree-core/editManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,8 @@ class SharedBranch<TEditor extends ChangeFamilyEditor, TChangeset> {
// Local changes, i.e., changes from this client are applied by fast forwarding the local branch commit onto
// the trunk.
if (areLocalCommits) {
for (const _ of newCommits) {
this.sequenceLocalCommit(nextSequenceId, sessionId, onSequenceLocalCommit);
for (const { revision } of newCommits) {
this.sequenceLocalCommit(revision, nextSequenceId, sessionId, onSequenceLocalCommit);
nextSequenceId = getNextSequenceId(nextSequenceId);
}
return;
Expand Down Expand Up @@ -896,10 +896,17 @@ class SharedBranch<TEditor extends ChangeFamilyEditor, TChangeset> {
* Avoiding the overhead of the rebase process, even when it's a no-op, has real measured performance benefits and is worth the added complexity here.
*/
private sequenceLocalCommit(
revision: RevisionTag,
sequenceId: SequenceId,
sessionId: SessionId,
onSequenceLocalCommit: OnSequenceCommit<TChangeset>,
): GraphCommit<TChangeset> {
): void {
if (this.commitMetadata.has(revision)) {
// This can happen if the commit came from a shared branch and was concurrently merged by another client.
// In this case, the newly sequenced commit is redundant and should therefore be ignored.
return;
}

// First, push the local commit to the trunk.
// We are mutating our `localCommits` cache here,but there is no need to actually change the `localBranch` itself because it will simply catch up later if/when it next rebases.
const firstLocalCommit = this.localCommits.shift();
Expand All @@ -911,7 +918,6 @@ class SharedBranch<TEditor extends ChangeFamilyEditor, TChangeset> {
const prevSequenceId = this.getCommitSequenceId(this.trunk.getHead().revision);
this.pushGraphCommitToTrunk(sequenceId, firstLocalCommit, sessionId);
onSequenceLocalCommit(firstLocalCommit, sequenceId, prevSequenceId);
return firstLocalCommit;
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/dds/tree/src/shared-tree-core/sharedTreeCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ export class SharedTreeCore<TEditor extends ChangeFamilyEditor, TChange>
this.editManager.localSessionId,
newRevision,
this.detachedRevision,
branchId,
);
this.editManager.advanceMinimumSequenceNumber(newRevision, false);
return undefined;
Expand Down
11 changes: 10 additions & 1 deletion packages/dds/tree/src/test/shared-tree/fuzz/baseModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ import type { Operation } from "./operationTypes.js";
import { takeAsync } from "@fluid-private/stochastic-test-utils";
import { type EditGeneratorOpWeights, makeOpGenerator } from "./fuzzEditGenerators.js";
import { FluidClientVersion } from "../../../codec/index.js";
import { ForestTypeOptimized, ForestTypeReference } from "../../../shared-tree/index.js";
import {
ForestTypeOptimized,
ForestTypeReference,
SharedTreeFormatVersion,
} from "../../../shared-tree/index.js";

export const runsPerBatch = 50;
// TODO: Enable other types of ops.
Expand All @@ -31,6 +35,10 @@ const editGeneratorOpWeights: Partial<EditGeneratorOpWeights> = {
nodeConstraint: 3,
fork: 1,
merge: 1,
createSharedBranch: 6,
checkoutSharedBranch: 9,
checkoutMainBranch: 6,
mergeSharedBranch: 6,
};
const generatorFactory = () => takeAsync(100, makeOpGenerator(editGeneratorOpWeights));

Expand All @@ -42,6 +50,7 @@ export const baseTreeModel: DDSFuzzModel<
workloadName: "SharedTree (Reference Forest)",
factory: new SharedTreeFuzzTestFactory(createOnCreate(undefined), undefined, {
oldestCompatibleClient: FluidClientVersion.EnableUnstableFeatures,
formatVersion: SharedTreeFormatVersion.vSharedBranches,
forest: ForestTypeReference,
}),
generatorFactory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
applyFieldEdit,
applySynchronizationOp,
applyUndoRedoEdit,
applySharedBranchOperation,
} from "./fuzzEditReducers.js";
import {
createOnCreate,
Expand Down Expand Up @@ -88,6 +89,9 @@ const fuzzComposedVsIndividualReducer = combineReducers<Operation, BranchedTreeF
forkMergeOperation: (state, operation) => {
return applyForkMergeOperation(state, operation);
},
sharedBranchOperation: (state, operation) => {
applySharedBranchOperation(state, operation);
},
});

/**
Expand Down
139 changes: 117 additions & 22 deletions packages/dds/tree/src/test/shared-tree/fuzz/fuzzEditGenerators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import { strict as assert } from "node:assert";

import { fail } from "@fluidframework/core-utils/internal";
import {
type AsyncGenerator,
type BaseFuzzTestState,
Expand Down Expand Up @@ -55,6 +56,8 @@ import {
GeneratedFuzzValueType,
type NodeRange,
type ForkMergeOperation,
type SharedBranchOperation,
type SharedBranchNumber,
} from "./operationTypes.js";
// eslint-disable-next-line import/no-internal-modules
import type { SchematizingSimpleTreeView } from "../../../shared-tree/schematizingTreeView.js";
Expand Down Expand Up @@ -121,6 +124,11 @@ export interface FuzzTestState extends DDSFuzzTestState<IChannelFactory<ISharedT
* which should be used in place of this view until the transaction is complete.
*/
forkedViews?: Map<ISharedTree, FuzzView[]>;

sharedBranchIdToNumber?: Map<string, SharedBranchNumber>;
sharedBranchNumberToId?: Map<SharedBranchNumber, string>;
activeSharedBranch?: Map<ISharedTree, { branchNumber: SharedBranchNumber; view: FuzzView }>;
sharedBranchViews?: Map<ISharedTree, Map<SharedBranchNumber, FuzzView>>;
}

export function viewFromState(
Expand All @@ -140,31 +148,39 @@ export function viewFromState(
return forkedViews[forkedBranchIndex];
}

const view =
state.transactionViews?.get(client.channel) ??
(getOrCreate(state.clientViews, client.channel, (sharedTree) => {
const tree = sharedTree.kernel;
const treeSchema = simpleSchemaFromStoredSchema(tree.storedSchema);
const config = new TreeViewConfiguration({
schema: treeSchema,
});
const transactionView = state.transactionViews?.get(client.channel);
if (transactionView !== undefined) {
return transactionView;
}

const treeView = asAlpha(tree.viewWith(config));
treeView.events.on("schemaChanged", () => {
if (!treeView.compatibility.canView) {
treeView.dispose();
state.clientViews?.delete(client.channel);
}
});
const sharedBranchView = state.activeSharedBranch?.get(client.channel);
if (sharedBranchView !== undefined) {
return sharedBranchView.view;
}

const view = getOrCreate(state.clientViews, client.channel, (sharedTree) => {
const tree = sharedTree.kernel;
const treeSchema = simpleSchemaFromStoredSchema(tree.storedSchema);
const config = new TreeViewConfiguration({
schema: treeSchema,
});

const treeView = asAlpha(tree.viewWith(config));
treeView.events.on("schemaChanged", () => {
if (!treeView.compatibility.canView) {
treeView.dispose();
state.clientViews?.delete(client.channel);
}
});

assert(treeView.compatibility.isEquivalent);
const fuzzView = treeView as FuzzView;
assert.equal(fuzzView.currentSchema, undefined);
const nodeSchema = nodeSchemaFromTreeSchema(treeSchema);
assert(treeView.compatibility.isEquivalent);
const fuzzView = treeView as FuzzView;
assert.equal(fuzzView.currentSchema, undefined);
const nodeSchema = nodeSchemaFromTreeSchema(treeSchema);

fuzzView.currentSchema = nodeSchema ?? assert.fail("nodeSchema should not be undefined");
return fuzzView;
}) as unknown as FuzzView);
fuzzView.currentSchema = nodeSchema ?? assert.fail("nodeSchema should not be undefined");
return fuzzView;
}) as unknown as FuzzView;
return view;
}
function filterFuzzNodeSchemas(
Expand Down Expand Up @@ -279,6 +295,11 @@ export interface EditGeneratorOpWeights {
nodeConstraint: number;
fork: number;
merge: number;

createSharedBranch: number;
checkoutSharedBranch: number;
checkoutMainBranch: number;
mergeSharedBranch: number;
}
const defaultEditGeneratorOpWeights: EditGeneratorOpWeights = {
set: 0,
Expand All @@ -298,6 +319,10 @@ const defaultEditGeneratorOpWeights: EditGeneratorOpWeights = {
nodeConstraint: 0,
fork: 0,
merge: 0,
createSharedBranch: 0,
checkoutSharedBranch: 0,
checkoutMainBranch: 0,
mergeSharedBranch: 0,
};

export interface EditGeneratorOptions {
Expand Down Expand Up @@ -584,6 +609,65 @@ export const makeTransactionEditGenerator = (
]);
};

export const makeSharedBranchOpGenerator = (
opWeightsArg: Partial<EditGeneratorOpWeights>,
): Generator<SharedBranchOperation, FuzzTestState> => {
const opWeights = {
...defaultEditGeneratorOpWeights,
...opWeightsArg,
};

return createWeightedGenerator<SharedBranchOperation, FuzzTestState>([
[
(state) => ({
type: "sharedBranchOperation",
contents: {
type: "createSharedBranch",
branchNumber: (state.sharedBranchIdToNumber?.size ?? 0) + 1,
},
}),
opWeights.createSharedBranch,
],
[
(state) => ({
type: "sharedBranchOperation",
contents: {
type: "checkoutSharedBranch",
branchNumber:
state.sharedBranchIdToNumber?.get(
state.random.pick(state.client.channel.getSharedBranchIds()),
) ?? fail("Missing branch number"),
},
}),
opWeights.checkoutSharedBranch,
(state) => state.client.channel.getSharedBranchIds().length > 0,
],
[
{
type: "sharedBranchOperation",
contents: {
type: "checkoutMainBranch",
},
},
opWeights.checkoutMainBranch,
(state) => state.activeSharedBranch?.get(state.client.channel) !== undefined,
],
[
(state) => ({
type: "sharedBranchOperation",
contents: {
type: "mergeSharedBranch",
branchNumber: state.random.pick(
Array.from(state.sharedBranchViews?.get(state.client.channel)?.keys() ?? []),
),
},
}),
opWeights.mergeSharedBranch,
(state) => state.sharedBranchViews?.get(state.client.channel) !== undefined,
],
]);
};

export const makeBranchEditGenerator = (
opWeightsArg: Partial<EditGeneratorOpWeights>,
): Generator<ForkMergeOperation, FuzzTestState> => {
Expand Down Expand Up @@ -720,6 +804,10 @@ export function makeOpGenerator(
nodeConstraint,
fork,
merge,
checkoutMainBranch,
checkoutSharedBranch,
createSharedBranch,
mergeSharedBranch,
...others
} = weights;
// This assert will trigger when new weights are added to EditGeneratorOpWeights but this function has not been
Expand All @@ -728,6 +816,12 @@ export function makeOpGenerator(
const editWeight = sumWeights([insert, remove, intraFieldMove, crossFieldMove, set, clear]);
const transactionWeight = sumWeights([abort, commit, start]);
const undoRedoWeight = sumWeights([undo, redo]);
const sharedBranchOpWeight = sumWeights([
checkoutMainBranch,
checkoutSharedBranch,
createSharedBranch,
mergeSharedBranch,
]);
// Currently we only support node constraints, but this may be expanded in the future.
const constraintWeight = nodeConstraint;

Expand All @@ -750,6 +844,7 @@ export function makeOpGenerator(
(state: FuzzTestState) => viewFromState(state).checkout.transaction.isInProgress(),
],
[() => makeBranchEditGenerator(weights), weights.fork + weights.merge],
[() => makeSharedBranchOpGenerator(weights), sharedBranchOpWeight],
] as const
)
.filter(([, weight]) => weight > 0)
Expand Down
Loading
Loading