feat(shorebird_cli): warn on low link percentages (#1832)
This commit is contained in:
@@ -110,6 +110,17 @@ If this option is not provided, the version number will be determined from the p
|
||||
final HashFunction _hashFn;
|
||||
final IosArchiveDiffer _archiveDiffer;
|
||||
|
||||
// Link percentage that is considered the minimum for acceptable perfromance.
|
||||
// This was selected arbitrarily and may need to be adjusted.
|
||||
static const double minLinkPercentage = 50;
|
||||
|
||||
static String lowLinkPercentageWarning(double linkPercentage) {
|
||||
return '''
|
||||
${lightCyan.wrap('shorebird patch')} was only able to share ${linkPercentage.toStringAsFixed(2)}% of Dart code with the released app.
|
||||
This means the patched code may execute slower than expected.
|
||||
https://docs.shorebird.dev/status#ios_link_percentage''';
|
||||
}
|
||||
|
||||
@override
|
||||
Future<int> run() async {
|
||||
try {
|
||||
@@ -305,15 +316,19 @@ Current Flutter Revision: $currentFlutterRevision
|
||||
),
|
||||
);
|
||||
|
||||
double? percentLinked;
|
||||
final useLinker = AotTools.usesLinker(release.flutterRevision);
|
||||
if (useLinker) {
|
||||
final exitCode = await _runLinker(
|
||||
final (:exitCode, :linkPercentage) = await _runLinker(
|
||||
releaseArtifact: releaseArtifactFile,
|
||||
);
|
||||
|
||||
if (exitCode != ExitCode.success.code) {
|
||||
return exitCode;
|
||||
if (exitCode != ExitCode.success.code) return exitCode;
|
||||
|
||||
if (linkPercentage != null && linkPercentage < minLinkPercentage) {
|
||||
logger.warn(lowLinkPercentageWarning(linkPercentage));
|
||||
}
|
||||
percentLinked = linkPercentage;
|
||||
}
|
||||
|
||||
if (dryRun) {
|
||||
@@ -368,6 +383,8 @@ Current Flutter Revision: $currentFlutterRevision
|
||||
'🟠 Track: ${lightCyan.wrap('Staging')}'
|
||||
else
|
||||
'🟢 Track: ${lightCyan.wrap('Production')}',
|
||||
if (percentLinked != null)
|
||||
'''🔗 Running ${lightCyan.wrap(percentLinked.toStringAsFixed(1))}% on CPU''',
|
||||
];
|
||||
|
||||
logger.info(
|
||||
@@ -503,12 +520,12 @@ ${summary.join('\n')}
|
||||
buildProgress.complete();
|
||||
}
|
||||
|
||||
Future<int> _runLinker({required File releaseArtifact}) async {
|
||||
Future<_LinkResult> _runLinker({required File releaseArtifact}) async {
|
||||
final patch = File(_aotOutputPath);
|
||||
|
||||
if (!patch.existsSync()) {
|
||||
logger.err('Unable to find patch AOT file at ${patch.path}');
|
||||
return ExitCode.software.code;
|
||||
return (exitCode: ExitCode.software.code, linkPercentage: null);
|
||||
}
|
||||
|
||||
final analyzeSnapshot = File(
|
||||
@@ -519,7 +536,7 @@ ${summary.join('\n')}
|
||||
|
||||
if (!analyzeSnapshot.existsSync()) {
|
||||
logger.err('Unable to find analyze_snapshot at ${analyzeSnapshot.path}');
|
||||
return ExitCode.software.code;
|
||||
return (exitCode: ExitCode.software.code, linkPercentage: null);
|
||||
}
|
||||
|
||||
final genSnapshot = shorebirdArtifacts.getArtifactPath(
|
||||
@@ -527,8 +544,9 @@ ${summary.join('\n')}
|
||||
);
|
||||
|
||||
final linkProgress = logger.progress('Linking AOT files');
|
||||
double? linkPercentage;
|
||||
try {
|
||||
await aotTools.link(
|
||||
linkPercentage = await aotTools.link(
|
||||
base: releaseArtifact.path,
|
||||
patch: patch.path,
|
||||
analyzeSnapshot: analyzeSnapshot.path,
|
||||
@@ -539,14 +557,15 @@ ${summary.join('\n')}
|
||||
);
|
||||
} catch (error) {
|
||||
linkProgress.fail('Failed to link AOT files: $error');
|
||||
return ExitCode.software.code;
|
||||
return (exitCode: ExitCode.software.code, linkPercentage: null);
|
||||
}
|
||||
|
||||
linkProgress.complete();
|
||||
return ExitCode.success.code;
|
||||
return (exitCode: ExitCode.success.code, linkPercentage: linkPercentage);
|
||||
}
|
||||
}
|
||||
|
||||
typedef _LinkResult = ({int exitCode, double? linkPercentage});
|
||||
|
||||
/// {@template _ReadVersionException}
|
||||
/// Exception thrown when the release version cannot be determined.
|
||||
/// {@endtemplate}
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import 'dart:convert';
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:collection/collection.dart';
|
||||
import 'package:mason_logger/mason_logger.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:pub_semver/pub_semver.dart';
|
||||
@@ -130,7 +132,7 @@ class AotTools {
|
||||
}
|
||||
|
||||
/// Generate a link vmcode file from two AOT snapshots.
|
||||
Future<void> link({
|
||||
Future<double?> link({
|
||||
required String base,
|
||||
required String patch,
|
||||
required String analyzeSnapshot,
|
||||
@@ -139,6 +141,8 @@ class AotTools {
|
||||
required String outputPath,
|
||||
String? workingDirectory,
|
||||
}) async {
|
||||
const linkJson = 'link.json';
|
||||
final outputDir = p.dirname(outputPath);
|
||||
final linkerUsesGenSnapshot = await _linkerUsesGenSnapshot();
|
||||
final result = await _exec(
|
||||
[
|
||||
@@ -146,9 +150,13 @@ class AotTools {
|
||||
'--base=$base',
|
||||
'--patch=$patch',
|
||||
'--analyze-snapshot=$analyzeSnapshot',
|
||||
if (linkerUsesGenSnapshot) '--gen-snapshot=$genSnapshot',
|
||||
if (linkerUsesGenSnapshot) '--kernel=$kernel',
|
||||
'--output=$outputPath',
|
||||
if (linkerUsesGenSnapshot) ...[
|
||||
'--gen-snapshot=$genSnapshot',
|
||||
'--kernel=$kernel',
|
||||
'--reporter=json',
|
||||
'--redirect-to=${p.join(outputDir, linkJson)}',
|
||||
],
|
||||
],
|
||||
workingDirectory: workingDirectory,
|
||||
);
|
||||
@@ -156,6 +164,24 @@ class AotTools {
|
||||
if (result.exitCode != 0) {
|
||||
throw Exception('Failed to link: ${result.stderr}');
|
||||
}
|
||||
|
||||
return linkerUsesGenSnapshot
|
||||
? _extractLinkPercentage(File(p.join(workingDirectory!, linkJson)))
|
||||
: null;
|
||||
}
|
||||
|
||||
double? _extractLinkPercentage(File file) {
|
||||
if (!file.existsSync()) return null;
|
||||
final status = const LineSplitter()
|
||||
.convert(file.readAsStringSync())
|
||||
.map(json.decode)
|
||||
.cast<Map<String, dynamic>>()
|
||||
.toList();
|
||||
|
||||
final linkSuccess = status.firstWhereOrNull(
|
||||
(line) => line['type'] == 'link_success',
|
||||
);
|
||||
return linkSuccess?['link_percentage'] as double?;
|
||||
}
|
||||
|
||||
/// Whether the current analyze_snapshot executable supports the
|
||||
|
||||
@@ -101,10 +101,10 @@ packages:
|
||||
dependency: "direct dev"
|
||||
description:
|
||||
name: build_runner
|
||||
sha256: "581bacf68f89ec8792f5e5a0b2c4decd1c948e97ce659dc783688c8a88fbec21"
|
||||
sha256: "3ac61a79bfb6f6cc11f693591063a7f19a7af628dc52f141743edac5c16e8c22"
|
||||
url: "https://pub.dev"
|
||||
source: hosted
|
||||
version: "2.4.8"
|
||||
version: "2.4.9"
|
||||
build_runner_core:
|
||||
dependency: transitive
|
||||
description:
|
||||
@@ -269,10 +269,10 @@ packages:
|
||||
dependency: transitive
|
||||
description:
|
||||
name: frontend_server_client
|
||||
sha256: "408e3ca148b31c20282ad6f37ebfa6f4bdc8fede5b74bc2f08d9d92b55db3612"
|
||||
sha256: f64a0333a82f30b0cca061bc3d143813a486dc086b574bfb233b7c1372427694
|
||||
url: "https://pub.dev"
|
||||
source: hosted
|
||||
version: "3.2.0"
|
||||
version: "4.0.0"
|
||||
glob:
|
||||
dependency: transitive
|
||||
description:
|
||||
|
||||
@@ -350,7 +350,7 @@ flutter:
|
||||
workingDirectory: any(named: 'workingDirectory'),
|
||||
outputPath: any(named: 'outputPath'),
|
||||
),
|
||||
).thenAnswer((_) async {});
|
||||
).thenAnswer((_) async => null);
|
||||
when(() => aotTools.isGeneratePatchDiffBaseSupported())
|
||||
.thenAnswer((_) async => false);
|
||||
when(
|
||||
@@ -418,8 +418,9 @@ flutter:
|
||||
).thenAnswer((_) async {});
|
||||
when(() => doctor.iosCommandValidators).thenReturn([flutterValidator]);
|
||||
when(() => engineConfig.localEngine).thenReturn(null);
|
||||
when(() => ios.exportOptionsPlistFromArgs(argResults))
|
||||
.thenReturn(File('.'));
|
||||
when(
|
||||
() => ios.exportOptionsPlistFromArgs(argResults),
|
||||
).thenReturn(File('.'));
|
||||
when(flutterValidator.validate).thenAnswer((_) async => []);
|
||||
when(() => logger.confirm(any())).thenReturn(true);
|
||||
when(() => logger.progress(any())).thenReturn(progress);
|
||||
@@ -840,6 +841,7 @@ Please re-run the release command for this version or create a new release.'''),
|
||||
).thenAnswer((_) async {
|
||||
// Ensure we're using the correct flutter revision.
|
||||
expect(shorebirdEnv.flutterRevision, equals(preLinkerFlutterRevision));
|
||||
return null;
|
||||
});
|
||||
|
||||
setUpProjectRoot();
|
||||
@@ -1271,6 +1273,33 @@ Please re-run the release command for this version or create a new release.'''),
|
||||
).called(1);
|
||||
});
|
||||
});
|
||||
|
||||
group('when aot_tools returns a low link percentage', () {
|
||||
setUp(() {
|
||||
when(
|
||||
() => aotTools.link(
|
||||
base: any(named: 'base'),
|
||||
patch: any(named: 'patch'),
|
||||
analyzeSnapshot: any(named: 'analyzeSnapshot'),
|
||||
genSnapshot: any(named: 'genSnapshot'),
|
||||
kernel: any(named: 'kernel'),
|
||||
workingDirectory: any(named: 'workingDirectory'),
|
||||
outputPath: any(named: 'outputPath'),
|
||||
),
|
||||
).thenAnswer((_) async => PatchIosCommand.minLinkPercentage - 1);
|
||||
});
|
||||
|
||||
test('logs a warning', () async {
|
||||
await runWithOverrides(command.run);
|
||||
verify(
|
||||
() => logger.warn(
|
||||
PatchIosCommand.lowLinkPercentageWarning(
|
||||
PatchIosCommand.minLinkPercentage - 1,
|
||||
),
|
||||
),
|
||||
).called(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
group('when aot-tools supports generating patch diff base', () {
|
||||
|
||||
+2
-1
@@ -298,7 +298,7 @@ flutter:
|
||||
workingDirectory: any(named: 'workingDirectory'),
|
||||
outputPath: any(named: 'outputPath'),
|
||||
),
|
||||
).thenAnswer((_) async {});
|
||||
).thenAnswer((_) async => null);
|
||||
when(() => argResults['release-version']).thenReturn(version);
|
||||
when(() => argResults.rest).thenReturn([]);
|
||||
when(() => artifactManager.downloadFile(any())).thenAnswer((_) async {
|
||||
@@ -608,6 +608,7 @@ Please re-run the release command for this version or create a new release.'''),
|
||||
),
|
||||
).thenAnswer((_) async {
|
||||
expect(shorebirdEnv.flutterRevision, equals(preLinkerFlutterRevision));
|
||||
return null;
|
||||
});
|
||||
when(
|
||||
() => shorebirdProcess.run(
|
||||
|
||||
@@ -2,6 +2,7 @@ import 'dart:io';
|
||||
|
||||
import 'package:mason_logger/mason_logger.dart';
|
||||
import 'package:mocktail/mocktail.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:scoped/scoped.dart';
|
||||
import 'package:shorebird_cli/src/cache.dart';
|
||||
import 'package:shorebird_cli/src/executables/executables.dart';
|
||||
@@ -59,6 +60,7 @@ void main() {
|
||||
const genSnapshot = './path/to/gen_snapshot';
|
||||
const kernel = './path/to/kernel.dill';
|
||||
const outputPath = './path/to/out.vmcode';
|
||||
const linkJsonPath = './path/to/link.json';
|
||||
|
||||
test('throws Exception when process exits with non-zero code', () async {
|
||||
when(
|
||||
@@ -330,9 +332,81 @@ void main() {
|
||||
'--base=$base',
|
||||
'--patch=$patch',
|
||||
'--analyze-snapshot=$analyzeSnapshot',
|
||||
'--output=$outputPath',
|
||||
'--gen-snapshot=$genSnapshot',
|
||||
'--kernel=$kernel',
|
||||
'--reporter=json',
|
||||
'--redirect-to=$linkJsonPath',
|
||||
],
|
||||
workingDirectory: any(named: 'workingDirectory'),
|
||||
),
|
||||
).called(1);
|
||||
});
|
||||
|
||||
test('returns link percentage', () async {
|
||||
workingDirectory = Directory.systemTemp.createTempSync();
|
||||
when(
|
||||
() => process.run(
|
||||
aotToolsPath,
|
||||
['--version'],
|
||||
workingDirectory: any(named: 'workingDirectory'),
|
||||
),
|
||||
).thenAnswer((_) async {
|
||||
return const ShorebirdProcessResult(
|
||||
exitCode: 0,
|
||||
stdout: '0.0.1',
|
||||
stderr: '',
|
||||
);
|
||||
});
|
||||
when(
|
||||
() => process.run(
|
||||
aotToolsPath,
|
||||
any(that: contains('--gen-snapshot=$genSnapshot')),
|
||||
workingDirectory: any(named: 'workingDirectory'),
|
||||
),
|
||||
).thenAnswer(
|
||||
(_) async {
|
||||
File(p.join(workingDirectory.path, 'link.json'))
|
||||
.writeAsStringSync(
|
||||
'''
|
||||
{"type":"link_success","base_codes_length":3036,"patch_codes_length":3036,"base_code_size":861816,"patch_code_size":861816,"linked_code_size":860460,"link_percentage":99.8426578295135}
|
||||
{"type":"link_debug","message":"wrote vmcode file to out.vmcode"}''',
|
||||
);
|
||||
return const ShorebirdProcessResult(
|
||||
exitCode: 0,
|
||||
stdout: '',
|
||||
stderr: '',
|
||||
);
|
||||
},
|
||||
);
|
||||
await expectLater(
|
||||
runWithOverrides(
|
||||
() => aotTools.link(
|
||||
base: base,
|
||||
patch: patch,
|
||||
analyzeSnapshot: analyzeSnapshot,
|
||||
genSnapshot: genSnapshot,
|
||||
kernel: kernel,
|
||||
workingDirectory: workingDirectory.path,
|
||||
outputPath: outputPath,
|
||||
),
|
||||
),
|
||||
completion(equals(99.8426578295135)),
|
||||
);
|
||||
|
||||
verify(
|
||||
() => process.run(
|
||||
aotToolsPath,
|
||||
[
|
||||
'link',
|
||||
'--base=$base',
|
||||
'--patch=$patch',
|
||||
'--analyze-snapshot=$analyzeSnapshot',
|
||||
'--output=$outputPath',
|
||||
'--gen-snapshot=$genSnapshot',
|
||||
'--kernel=$kernel',
|
||||
'--reporter=json',
|
||||
'--redirect-to=$linkJsonPath',
|
||||
],
|
||||
workingDirectory: any(named: 'workingDirectory'),
|
||||
),
|
||||
|
||||
Reference in New Issue
Block a user