Skip to content

Commit

Permalink
refactor(migrations): update migrations to combine analysis data in p…
Browse files Browse the repository at this point in the history
…arallel (#58280)

This is necessary given the previous Tsurge refactorings. It should
speed up migrations in the merge phase signficiantly and reduce memory
pressure. In 1P, we already have workers from analyze phase anyway— so
those can be re-used for parallel metadata merging.

PR Close #58280
  • Loading branch information
devversion committed Oct 25, 2024
1 parent 8143016 commit dd686ed
Show file tree
Hide file tree
Showing 9 changed files with 174 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,10 @@ export class OutputMigration extends TsurgeFunnelMigration<
});
}

override async merge(units: CompilationUnitData[]): Promise<Serializable<CompilationUnitData>> {
override async combine(
unitA: CompilationUnitData,
unitB: CompilationUnitData,
): Promise<Serializable<CompilationUnitData>> {
const outputFields: Record<ClassFieldUniqueKey, OutputMigrationData> = {};
const importReplacements: Record<
ProjectFileID,
Expand All @@ -312,7 +315,7 @@ export class OutputMigration extends TsurgeFunnelMigration<
const problematicUsages: Record<ClassFieldUniqueKey, true> = {};
let problematicDeclarationCount = 0;

for (const unit of units) {
for (const unit of [unitA, unitB]) {
for (const declIdStr of Object.keys(unit.outputFields)) {
const declId = declIdStr as ClassFieldUniqueKey;
// THINK: detect clash? Should we have an utility to merge data based on unique IDs?
Expand All @@ -327,13 +330,10 @@ export class OutputMigration extends TsurgeFunnelMigration<
problematicDeclarationCount += unit.problematicDeclarationCount;
}

for (const unit of units) {
for (const unit of [unitA, unitB]) {
for (const declIdStr of Object.keys(unit.problematicUsages)) {
const declId = declIdStr as ClassFieldUniqueKey;
// it might happen that a problematic usage is detected but we didn't see the declaration - skipping those
if (outputFields[declId] !== undefined) {
problematicUsages[declId] = unit.problematicUsages[declId];
}
problematicUsages[declId] = unit.problematicUsages[declId];
}
}

Expand All @@ -345,6 +345,28 @@ export class OutputMigration extends TsurgeFunnelMigration<
});
}

override async globalMeta(
combinedData: CompilationUnitData,
): Promise<Serializable<CompilationUnitData>> {
const globalMeta: CompilationUnitData = {
importReplacements: combinedData.importReplacements,
outputFields: combinedData.outputFields,
problematicDeclarationCount: combinedData.problematicDeclarationCount,
problematicUsages: {},
};

for (const keyStr of Object.keys(combinedData.problematicUsages)) {
const key = keyStr as ClassFieldUniqueKey;
// it might happen that a problematic usage is detected but we didn't see the declaration - skipping those
if (globalMeta.outputFields[key] !== undefined) {
globalMeta.problematicUsages[key] = true;
}
}

// Noop here as we don't have any form of special global metadata.
return confirmAsSerializable(combinedData);
}

override async stats(globalMetadata: CompilationUnitData): Promise<MigrationStats> {
const detectedOutputs =
new Set(Object.keys(globalMetadata.outputFields)).size +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,19 @@ import {CompilationUnitData} from './unit_data';
type InputData = {key: string; info: CompilationUnitData['knownInputs'][string]};

/** Merges a list of compilation units into a combined unit. */
export function mergeCompilationUnitData(
metadataFiles: CompilationUnitData[],
export function combineCompilationUnitData(
unitA: CompilationUnitData,
unitB: CompilationUnitData,
): CompilationUnitData {
const result: CompilationUnitData = {
knownInputs: {},
};

const idToGraphNode = new Map<string, GraphNode<InputData>>();
const inheritanceGraph: GraphNode<InputData>[] = [];
const isNodeIncompatible = (node: InputData) =>
node.info.memberIncompatibility !== null || node.info.owningClassIncompatibility !== null;

for (const file of metadataFiles) {
for (const file of [unitA, unitB]) {
for (const [key, info] of Object.entries(file.knownInputs)) {
const existing = result.knownInputs[key];
if (existing === undefined) {
result.knownInputs[key] = info;
const node: GraphNode<InputData> = {
incoming: new Set(),
outgoing: new Set(),
data: {info, key},
};
inheritanceGraph.push(node);
idToGraphNode.set(key, node);
continue;
}

Expand Down Expand Up @@ -77,7 +66,37 @@ export function mergeCompilationUnitData(
}
}

for (const [key, info] of Object.entries(result.knownInputs)) {
return result;
}

export function convertToGlobalMeta(combinedData: CompilationUnitData): CompilationUnitData {
const globalMeta: CompilationUnitData = {
knownInputs: {},
};

const idToGraphNode = new Map<string, GraphNode<InputData>>();
const inheritanceGraph: GraphNode<InputData>[] = [];
const isNodeIncompatible = (node: InputData) =>
node.info.memberIncompatibility !== null || node.info.owningClassIncompatibility !== null;

for (const [key, info] of Object.entries(combinedData.knownInputs)) {
const existing = globalMeta.knownInputs[key];
if (existing !== undefined) {
continue;
}

const node: GraphNode<InputData> = {
incoming: new Set(),
outgoing: new Set(),
data: {info, key},
};
inheritanceGraph.push(node);
idToGraphNode.set(key, node);

globalMeta.knownInputs[key] = info;
}

for (const [key, info] of Object.entries(globalMeta.knownInputs)) {
if (info.extendsFrom !== null) {
const from = idToGraphNode.get(key)!;
const target = idToGraphNode.get(info.extendsFrom)!;
Expand Down Expand Up @@ -109,7 +128,7 @@ export function mergeCompilationUnitData(
}
}

for (const info of Object.values(result.knownInputs)) {
for (const info of Object.values(combinedData.knownInputs)) {
// We never saw a source file for this input, globally. Try marking it as incompatible,
// so that all references and inheritance checks can propagate accordingly.
if (!info.seenAsSourceInput) {
Expand All @@ -125,5 +144,5 @@ export function mergeCompilationUnitData(
}
}

return result;
return globalMeta;
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@
import fs from 'fs';
import path from 'path';
import {executeAnalyzePhase} from '../../../../utils/tsurge/executors/analyze_exec';
import {executeMergePhase} from '../../../../utils/tsurge/executors/merge_exec';
import {executeCombinePhase} from '../../../../utils/tsurge/executors/combine_exec';
import {executeMigratePhase} from '../../../../utils/tsurge/executors/migrate_exec';
import {SignalInputMigration} from '../migration';
import {writeMigrationReplacements} from '../write_replacements';
import {CompilationUnitData} from './unit_data';
import {executeGlobalMetaPhase} from '../../../../utils/tsurge/executors/global_meta_exec';

main().catch((e) => {
console.error(e);
Expand All @@ -27,19 +28,16 @@ async function main() {
if (mode === 'extract') {
const analyzeResult = await executeAnalyzePhase(migration, path.resolve(args[0]));
process.stdout.write(JSON.stringify(analyzeResult));
} else if (mode === 'merge') {
const mergedResult = await executeMergePhase(
migration,
await Promise.all(
args.map((p) =>
fs.promises
.readFile(path.resolve(p), 'utf8')
.then((data) => JSON.parse(data) as CompilationUnitData),
),
),
);
} else if (mode === 'combine') {
const unitAPromise = readUnitMeta(path.resolve(args[0]));
const unitBPromise = readUnitMeta(path.resolve(args[1]));

const [unitA, unitB] = await Promise.all([unitAPromise, unitBPromise]);
const mergedResult = await executeCombinePhase(migration, unitA, unitB);

process.stdout.write(JSON.stringify(mergedResult));
} else if (mode === 'globalMeta') {
await executeGlobalMetaPhase(migration, await readUnitMeta(args[0]));
} else if (mode === 'migrate') {
const {replacements, projectRoot} = await executeMigratePhase(
migration,
Expand All @@ -50,3 +48,9 @@ async function main() {
writeMigrationReplacements(replacements, projectRoot);
}
}

async function readUnitMeta(filePath: string): Promise<CompilationUnitData> {
return fs.promises
.readFile(filePath, 'utf8')
.then((data) => JSON.parse(data) as CompilationUnitData);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {MigrationHost} from './migration_host';
import {executeAnalysisPhase} from './phase_analysis';
import {pass4__checkInheritanceOfInputs} from './passes/4_check_inheritance';
import {getCompilationUnitMetadata} from './batch/extract';
import {mergeCompilationUnitData} from './batch/merge_unit_data';
import {convertToGlobalMeta, combineCompilationUnitData} from './batch/merge_unit_data';
import {Replacement} from '../../../utils/tsurge/replacement';
import {populateKnownInputsFromGlobalData} from './batch/populate_global_data';
import {executeMigrationPhase} from './phase_migrate';
Expand All @@ -28,10 +28,8 @@ import {
ClassIncompatibilityReason,
FieldIncompatibilityReason,
} from './passes/problematic_patterns/incompatibility';
import {isInputDescriptor} from './utils/input_id';
import {MigrationConfig} from './migration_config';
import {ClassFieldUniqueKey} from './passes/reference_resolution/known_fields';
import {MigrationStats} from '../../../utils/tsurge';
import {createNgtscProgram} from '../../../utils/tsurge/helpers/ngtsc_program';

/**
Expand Down Expand Up @@ -124,8 +122,8 @@ export class SignalInputMigration extends TsurgeComplexMigration<

// Non-batch mode!
if (this.config.upgradeAnalysisPhaseToAvoidBatch) {
const merged = await this.merge([unitData]);
const {replacements} = await this.migrate(merged, info, {
const globalMeta = await this.globalMeta(unitData);
const {replacements} = await this.migrate(globalMeta, info, {
knownInputs,
result,
host,
Expand All @@ -143,8 +141,17 @@ export class SignalInputMigration extends TsurgeComplexMigration<
return confirmAsSerializable(unitData);
}

override async merge(units: CompilationUnitData[]): Promise<Serializable<CompilationUnitData>> {
return confirmAsSerializable(mergeCompilationUnitData(units));
override async combine(
unitA: CompilationUnitData,
unitB: CompilationUnitData,
): Promise<Serializable<CompilationUnitData>> {
return confirmAsSerializable(combineCompilationUnitData(unitA, unitB));
}

override async globalMeta(
combinedData: CompilationUnitData,
): Promise<Serializable<CompilationUnitData>> {
return confirmAsSerializable(convertToGlobalMeta(combinedData));
}

override async migrate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,52 +309,84 @@ export class SignalQueriesMigration extends TsurgeComplexMigration<
return confirmAsSerializable(res);
}

override async merge(units: CompilationUnitData[]): Promise<Serializable<GlobalUnitData>> {
const merged: GlobalUnitData = {
override async combine(
unitA: CompilationUnitData,
unitB: CompilationUnitData,
): Promise<Serializable<CompilationUnitData>> {
const combined: CompilationUnitData = {
knownQueryFields: {},
problematicQueries: {},
potentialProblematicQueries: {},
potentialProblematicReferenceForMultiQueries: {},
reusableAnalysisReferences: null,
};

for (const unit of units) {
for (const unit of [unitA, unitB]) {
for (const [id, value] of Object.entries(unit.knownQueryFields)) {
merged.knownQueryFields[id as ClassFieldUniqueKey] = value;
combined.knownQueryFields[id as ClassFieldUniqueKey] = value;
}

for (const [id, info] of Object.entries(unit.potentialProblematicQueries)) {
if (info.fieldReason !== null) {
markFieldIncompatibleInMetadata(
merged.problematicQueries,
combined.potentialProblematicQueries,
id as ClassFieldUniqueKey,
info.fieldReason,
);
}
if (info.classReason !== null) {
merged.problematicQueries[id as ClassFieldUniqueKey] ??= {
combined.potentialProblematicQueries[id as ClassFieldUniqueKey] ??= {
classReason: null,
fieldReason: null,
};
merged.problematicQueries[id as ClassFieldUniqueKey].classReason = info.classReason;
combined.potentialProblematicQueries[id as ClassFieldUniqueKey].classReason =
info.classReason;
}
}

for (const id of Object.keys(unit.potentialProblematicReferenceForMultiQueries)) {
combined.potentialProblematicReferenceForMultiQueries[id as ClassFieldUniqueKey] = true;
}

if (unit.reusableAnalysisReferences !== null) {
assert(units.length === 1, 'Expected migration to not run in batch mode');
merged.reusableAnalysisReferences = unit.reusableAnalysisReferences;
combined.reusableAnalysisReferences = unit.reusableAnalysisReferences;
}
}

for (const unit of units) {
for (const unit of [unitA, unitB]) {
for (const id of Object.keys(unit.potentialProblematicReferenceForMultiQueries)) {
if (merged.knownQueryFields[id as ClassFieldUniqueKey]?.isMulti) {
if (combined.knownQueryFields[id as ClassFieldUniqueKey]?.isMulti) {
markFieldIncompatibleInMetadata(
merged.problematicQueries,
combined.potentialProblematicQueries,
id as ClassFieldUniqueKey,
FieldIncompatibilityReason.SignalQueries__QueryListProblematicFieldAccessed,
);
}
}
}

return confirmAsSerializable(merged);
return confirmAsSerializable(combined);
}

override async globalMeta(
combinedData: CompilationUnitData,
): Promise<Serializable<GlobalUnitData>> {
const globalUnitData: GlobalUnitData = {
knownQueryFields: combinedData.knownQueryFields,
problematicQueries: combinedData.potentialProblematicQueries,
reusableAnalysisReferences: combinedData.reusableAnalysisReferences,
};

for (const id of Object.keys(combinedData.potentialProblematicReferenceForMultiQueries)) {
if (combinedData.knownQueryFields[id as ClassFieldUniqueKey]?.isMulti) {
markFieldIncompatibleInMetadata(
globalUnitData.problematicQueries,
id as ClassFieldUniqueKey,
FieldIncompatibilityReason.SignalQueries__QueryListProblematicFieldAccessed,
);
}
}

return confirmAsSerializable(globalUnitData);
}

override async migrate(globalMetadata: GlobalUnitData, info: ProgramInfo) {
Expand Down
13 changes: 10 additions & 3 deletions packages/core/schematics/ng-generate/output-migration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
OutputMigration,
} from '../../migrations/output-migration/output-migration';
import {ProjectRootRelativePath, TextUpdate} from '../../utils/tsurge';
import {synchronouslyCombineUnitData} from '../../utils/tsurge/helpers/combine_units';

interface Options {
path: string;
Expand Down Expand Up @@ -75,13 +76,19 @@ export function migrate(options: Options): Rule {
context.logger.info(`Processing analysis data between targets..`);
context.logger.info(``);

const merged = await migration.merge(unitResults);
const combined = await synchronouslyCombineUnitData(migration, unitResults);
if (combined === null) {
context.logger.error('Migration failed unexpectedly with no analysis data');
return;
}

const globalMeta = await migration.globalMeta(combined);
const replacementsPerFile: Map<ProjectRootRelativePath, TextUpdate[]> = new Map();

for (const {info, tsconfigPath} of programInfos) {
context.logger.info(`Migrating: ${tsconfigPath}..`);

const {replacements} = await migration.migrate(merged);
const {replacements} = await migration.migrate(globalMeta);
const changesPerFile = groupReplacementsByFile(replacements);

for (const [file, changes] of changesPerFile) {
Expand All @@ -104,7 +111,7 @@ export function migrate(options: Options): Rule {

const {
counters: {detectedOutputs, problematicOutputs, successRate},
} = await migration.stats(merged);
} = await migration.stats(globalMeta);
const migratedOutputs = detectedOutputs - problematicOutputs;
const successRatePercent = (successRate * 100).toFixed(2);

Expand Down
Loading

0 comments on commit dd686ed

Please sign in to comment.