diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart index 7f1a5f0d..049b3f65 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart @@ -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 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 _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} diff --git a/packages/shorebird_cli/lib/src/executables/aot_tools.dart b/packages/shorebird_cli/lib/src/executables/aot_tools.dart index c15530a1..ba53833b 100644 --- a/packages/shorebird_cli/lib/src/executables/aot_tools.dart +++ b/packages/shorebird_cli/lib/src/executables/aot_tools.dart @@ -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 link({ + Future 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>() + .toList(); + + final linkSuccess = status.firstWhereOrNull( + (line) => line['type'] == 'link_success', + ); + return linkSuccess?['link_percentage'] as double?; } /// Whether the current analyze_snapshot executable supports the diff --git a/packages/shorebird_cli/pubspec.lock b/packages/shorebird_cli/pubspec.lock index c3740034..cc295bf4 100644 --- a/packages/shorebird_cli/pubspec.lock +++ b/packages/shorebird_cli/pubspec.lock @@ -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: diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart index f2b15023..e726a57d 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart @@ -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', () { diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart index 8f78a1ba..425fa33e 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart @@ -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( diff --git a/packages/shorebird_cli/test/src/executables/aot_tools_test.dart b/packages/shorebird_cli/test/src/executables/aot_tools_test.dart index 4a5908cd..ea9d9349 100644 --- a/packages/shorebird_cli/test/src/executables/aot_tools_test.dart +++ b/packages/shorebird_cli/test/src/executables/aot_tools_test.dart @@ -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'), ),