Skip to content

Commit

Permalink
Revert "refactor(migrations): update migrations to combine analysis d…
Browse files Browse the repository at this point in the history
…ata in parallel (angular#58280)"

This reverts commit 8735543.
  • Loading branch information
AndrewKushnir committed Oct 22, 2024
1 parent 52b5e59 commit 30525e0
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 147 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -291,18 +291,15 @@ export class OutputMigration extends TsurgeFunnelMigration<
});
}

override async combine(
unitA: CompilationUnitData,
unitB: CompilationUnitData,
): Promise<Serializable<CompilationUnitData>> {
override async merge(units: CompilationUnitData[]): Promise<Serializable<CompilationUnitData>> {
const outputFields: Record<ClassFieldUniqueKey, OutputMigrationData> = {};
const importReplacements: Record<
ProjectFileID,
{add: Replacement[]; addAndRemove: Replacement[]}
> = {};
const problematicUsages: Record<ClassFieldUniqueKey, true> = {};

for (const unit of [unitA, unitB]) {
for (const unit of units) {
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 +324,6 @@ export class OutputMigration extends TsurgeFunnelMigration<
});
}

override async globalMeta(
combinedData: CompilationUnitData,
): Promise<Serializable<CompilationUnitData>> {
// Noop here as we don't have any form of special global metadata.
return confirmAsSerializable(combinedData);
}

override async stats(globalMetadata: CompilationUnitData): Promise<MigrationStats> {
// TODO: Add statistics.
return {counters: {}};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,30 @@ 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 combineCompilationUnitData(
unitA: CompilationUnitData,
unitB: CompilationUnitData,
export function mergeCompilationUnitData(
metadataFiles: CompilationUnitData[],
): CompilationUnitData {
const result: CompilationUnitData = {
knownInputs: {},
};

for (const file of [unitA, unitB]) {
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 [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 @@ -66,37 +77,7 @@ export function combineCompilationUnitData(
}
}

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)) {
for (const [key, info] of Object.entries(result.knownInputs)) {
if (info.extendsFrom !== null) {
const from = idToGraphNode.get(key)!;
const target = idToGraphNode.get(info.extendsFrom)!;
Expand Down Expand Up @@ -128,7 +109,7 @@ export function convertToGlobalMeta(combinedData: CompilationUnitData): Compilat
}
}

for (const info of Object.values(combinedData.knownInputs)) {
for (const info of Object.values(result.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 @@ -144,5 +125,5 @@ export function convertToGlobalMeta(combinedData: CompilationUnitData): Compilat
}
}

return globalMeta;
return result;
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@
import fs from 'fs';
import path from 'path';
import {executeAnalyzePhase} from '../../../../utils/tsurge/executors/analyze_exec';
import {executeCombinePhase} from '../../../../utils/tsurge/executors/combine_exec';
import {executeMergePhase} from '../../../../utils/tsurge/executors/merge_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 @@ -28,16 +27,19 @@ async function main() {
if (mode === 'extract') {
const analyzeResult = await executeAnalyzePhase(migration, path.resolve(args[0]));
process.stdout.write(JSON.stringify(analyzeResult));
} 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);
} 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),
),
),
);

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 @@ -48,9 +50,3 @@ 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 {convertToGlobalMeta, combineCompilationUnitData} from './batch/merge_unit_data';
import {mergeCompilationUnitData} 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,8 +28,10 @@ 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 @@ -122,8 +124,8 @@ export class SignalInputMigration extends TsurgeComplexMigration<

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

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 merge(units: CompilationUnitData[]): Promise<Serializable<CompilationUnitData>> {
return confirmAsSerializable(mergeCompilationUnitData(units));
}

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

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

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

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

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

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

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

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);
return confirmAsSerializable(merged);
}

override async migrate(globalMetadata: GlobalUnitData, info: ProgramInfo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {groupReplacementsByFile} from '../../utils/tsurge/helpers/group_replacem
import {setFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system';
import {CompilationUnitData} from '../../migrations/signal-migration/src/batch/unit_data';
import {ProjectRootRelativePath, TextUpdate} from '../../utils/tsurge';
import {synchronouslyCombineUnitData} from '../../utils/tsurge/helpers/combine_units';

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

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 merged = await migration.merge(unitResults);
const replacementsPerFile: Map<ProjectRootRelativePath, TextUpdate[]> = new Map();

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

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

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

const {counters} = await migration.stats(globalMeta);
const {counters} = await migration.stats(merged);
const migratedInputs = counters.sourceInputs - counters.incompatibleInputs;

context.logger.info('');
Expand Down
Loading

0 comments on commit 30525e0

Please sign in to comment.