Skip to content

Adds script to run malioc locally#185371

Merged
gaaclarke merged 5 commits into
flutter:masterfrom
gaaclarke:malioc-script
Apr 22, 2026
Merged

Adds script to run malioc locally#185371
gaaclarke merged 5 commits into
flutter:masterfrom
gaaclarke:malioc-script

Conversation

@gaaclarke

@gaaclarke gaaclarke commented Apr 21, 2026

Copy link
Copy Markdown
Member

adds script to help running malioc locally

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@flutter-dashboard

flutter-dashboard Bot commented Apr 21, 2026

Copy link
Copy Markdown

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@gaaclarke gaaclarke requested a review from b-luk April 21, 2026 20:49
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Apr 21, 2026
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Apr 21, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Dart script, malioc_download_and_diff.dart, which automates the process of downloading Mali Offline Compiler tools, executing GN and Ninja builds, and comparing shader performance statistics against a golden file. The review feedback identifies a bug where calling exit() in helper functions prevents the execution of the finally block responsible for cleaning up temporary directories. Additionally, the feedback suggests replacing a hardcoded fallback version string with an error and using platform-agnostic logic to identify the malioc executable.

Comment thread engine/src/flutter/impeller/tools/malioc_download_and_diff.dart Outdated
Comment thread engine/src/flutter/impeller/tools/malioc_download_and_diff.dart
Future<String> getArmToolsVersion(Directory flutterDir) async {
final jsonFile = File('${flutterDir.path}/ci/builders/linux_unopt.json');
if (!await jsonFile.exists()) {
return 'last_updated:2023-02-03T15:32:01-0800';

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The hardcoded version string 'last_updated:2023-02-03T15:32:01-0800' is used as a fallback if the configuration file is missing or the dependency is not found. This is fragile and likely to become stale. It would be better to throw an error to notify the user that the required version information could not be found in the expected configuration file.

Comment thread engine/src/flutter/impeller/tools/malioc_download_and_diff.dart
Future<String> findMalioc(Directory armToolsDir) async {
if (await armToolsDir.exists()) {
await for (final entity in armToolsDir.list(recursive: true)) {
if (entity is File && entity.path.endsWith('/malioc')) {

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The check entity.path.endsWith('/malioc') is platform-specific and may fail on Windows. To improve portability, consider checking the file name using entity.uri.pathSegments.last or using Platform.pathSeparator.

Suggested change
if (entity is File && entity.path.endsWith('/malioc')) {
if (entity is File && entity.uri.pathSegments.last == 'malioc') {

b-luk
b-luk reviewed Apr 21, 2026
View reviewed changes
Comment thread engine/src/flutter/impeller/tools/malioc_download_and_diff.dart Outdated
bool update = false;
String? maliocPath;

for (int i = 0; i < arguments.length; i++) {

@b-luk b-luk Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ArgParser()?

@gaaclarke gaaclarke Apr 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would add a dependency which complicates this usage. Right now it's just a script.

@b-luk b-luk Apr 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see lots of uses of this for other scripts: https://github.com/search?q=repo%3Aflutter%2Fflutter+argparser&type=code. I think ArgParse is something that only makes sense when used in a script, so that's kind of exactly what it's for.

I think it makes the parsing look much more readable, but I'll leave it up to you.

@gaaclarke gaaclarke Apr 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I say "script" i mean a dart file you can execute. Those other things are dart projects with their own pubspec's. It complicates the setup a bit.

@b-luk b-luk Apr 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think running this script would use the pubspec.yaml in flutter/engine/src/flutter, which already specifies an args dependency:

flutter/engine/src/flutter/pubspec.yaml

Line 127 in 2000a5a

args: any

So I think it would just work. I tested it out by putting a simple dart script in this directory that calls import 'package:args/args.dart';, and it seems to work.

@gaaclarke gaaclarke Apr 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorcery! TIL

Comment thread engine/src/flutter/impeller/tools/malioc_download_and_diff.dart Outdated
Comment thread engine/src/flutter/impeller/tools/malioc_download_and_diff.dart Outdated
}

print('Downloading malioc tools to ${armToolsDir.path}...');
final cipdProcess = await Process.start('cipd', [

@b-luk b-luk Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This downloads 850MB. Do we really want to make the default way to run this tool be to download this and then immediately delete it after? Maybe we should be more forceful about making the user provide a directory, and automatically download it there if it doesn't exist.

@gaaclarke gaaclarke Apr 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This duplicates how it is run on ci, someone can bypass it with --malioc-path. It seems like there are tradeoffs however we cut it. Maybe just keeping it in alignment with ci is the good default.

@b-luk b-luk Apr 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think running on CI from a blank state is quite a different scenario than a developer running it in their environment. But not a big deal. SGTM.

Comment thread engine/src/flutter/impeller/tools/malioc_download_and_diff.dart
@gaaclarke gaaclarke added the CICD Run CI/CD label Apr 21, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label Apr 21, 2026
@gaaclarke gaaclarke added the CICD Run CI/CD label Apr 21, 2026
b-luk
b-luk approved these changes Apr 22, 2026
View reviewed changes
Comment thread engine/src/flutter/impeller/tools/malioc_download_and_diff.dart Outdated
bool update = false;
String? maliocPath;

for (int i = 0; i < arguments.length; i++) {

@b-luk b-luk Apr 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see lots of uses of this for other scripts: https://github.com/search?q=repo%3Aflutter%2Fflutter+argparser&type=code. I think ArgParse is something that only makes sense when used in a script, so that's kind of exactly what it's for.

I think it makes the parsing look much more readable, but I'll leave it up to you.

Comment thread engine/src/flutter/impeller/tools/malioc_download_and_diff.dart Outdated
}

print('Downloading malioc tools to ${armToolsDir.path}...');
final cipdProcess = await Process.start('cipd', [

@b-luk b-luk Apr 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think running on CI from a blank state is quite a different scenario than a developer running it in their environment. But not a big deal. SGTM.

@gaaclarke gaaclarke added this pull request to the merge queue Apr 22, 2026
Merged via the queue into flutter:master with commit 3dfc2cb Apr 22, 2026
193 checks passed
@gaaclarke gaaclarke deleted the malioc-script branch April 22, 2026 23:17
This was referenced Apr 23, 2026
Closed
Closed
Merged
Closed
Closed
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Apr 24, 2026
@engine-flutter-autoroll
Roll Flutter from 5e4f16931847 to aeb96234de86 (42 revisions) (#11576)
6a92649 
Roll Flutter from 5e4f16931847 to aeb96234de86 (42 revisions)

flutter/flutter@5e4f169...aeb9623

2026-04-24 robert.ancell@canonical.com Fix leak on error case (flutter/flutter#185516)
2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from 04c6c369cfd0 to 1c9b0b9141e9 (2 revisions) (flutter/flutter#185529)
2026-04-24 engine-flutter-autoroll@skia.org Roll Dart SDK from f386b11262f6 to c26627715892 (1 revision) (flutter/flutter#185526)
2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from 290a056fcd0e to 04c6c369cfd0 (2 revisions) (flutter/flutter#185525)
2026-04-24 chris@bracken.jp [ios] Extract SplashScreenManager from FlutterViewController (flutter/flutter#185405)
2026-04-24 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from j3UCWZhWx7zSl9Asy... to 9fPnyEo9PaNdXtasl... (flutter/flutter#185523)
2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from 4c8bedd3c932 to 290a056fcd0e (1 revision) (flutter/flutter#185518)
2026-04-24 engine-flutter-autoroll@skia.org Roll Dart SDK from 70665fc3fd2e to f386b11262f6 (2 revisions) (flutter/flutter#185512)
2026-04-24 97480502+b-luk@users.noreply.github.com Handle hairline strokes in UberSDF (flutter/flutter#184895)
2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from ea20c73ac72c to 4c8bedd3c932 (3 revisions) (flutter/flutter#185509)
2026-04-24 58529443+srujzs@users.noreply.github.com Use relative path for reloadedSourcesUri and reloaded modules (flutter/flutter#184598)
2026-04-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Run all flutter/flutter macOS tests using Xcode 26 and iOS 26 simulator (#185431)" (flutter/flutter#185513)
2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from e8d00d634c22 to ea20c73ac72c (8 revisions) (flutter/flutter#185500)
2026-04-23 okorohelijah@google.com Run all flutter/flutter macOS tests using Xcode 26 and iOS 26 simulator (flutter/flutter#185431)
2026-04-23 rmolivares@renzo-olivares.dev update team-text-input pr triage link to filter out "waiting for response" label (flutter/flutter#185499)
2026-04-23 jason-simmons@users.noreply.github.com Check for overflow when computing the pixel buffer size for an animated PNG frame (flutter/flutter#185442)
2026-04-23 reinar@crypt.ws Impeller: Recreate Vulkan transients on surface size change (flutter/flutter#185122)
2026-04-23 matt.kosarek@canonical.com Updating the windowing API for sized to content regular and dialog windows + removing the decorated flag (flutter/flutter#184977)
2026-04-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 634991935e9a to 70665fc3fd2e (2 revisions) (flutter/flutter#185488)
2026-04-23 louisehsu@google.com Updating ios triage link (flutter/flutter#185437)
2026-04-23 34871572+gmackall@users.noreply.github.com Revert "Preprovision Android NDK for flavored builds and reuse matchi… (flutter/flutter#185439)
2026-04-23 1961493+harryterkelsen@users.noreply.github.com fix(web): Fix LateInitializationError in CkSurface and SkwasmSurface (flutter/flutter#185116)
2026-04-23 1961493+harryterkelsen@users.noreply.github.com [web] Implement stepped image downscaling for CanvasKit and Skwasm (flutter/flutter#184741)
2026-04-23 30870216+gaaclarke@users.noreply.github.com Made wide_gamut_macos only run on arm64 machines (flutter/flutter#185486)
2026-04-23 chris@bracken.jp [ios] Update documentation for FlutterAppDelegate.pluginRegistrant (flutter/flutter#185201)
2026-04-23 engine-flutter-autoroll@skia.org Roll Dart SDK from bdf48933f3cf to 634991935e9a (1 revision) (flutter/flutter#185462)
2026-04-23 engine-flutter-autoroll@skia.org Roll Skia from 5fe6162546b1 to e8d00d634c22 (3 revisions) (flutter/flutter#185459)
2026-04-23 engine-flutter-autoroll@skia.org Roll Skia from 0049c5d91b08 to 5fe6162546b1 (1 revision) (flutter/flutter#185455)
2026-04-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 9648f446f131 to bdf48933f3cf (19 revisions) (flutter/flutter#185451)
2026-04-23 engine-flutter-autoroll@skia.org Roll Skia from 11640d1cbc5c to 0049c5d91b08 (11 revisions) (flutter/flutter#185453)
2026-04-23 ishaquehassan@gmail.com Add disposal guidance to CurvedAnimation and CurveTween docs (flutter/flutter#184569)
2026-04-23 ishaquehassan@gmail.com Add `clipBehavior` parameter to `AnimatedCrossFade` (flutter/flutter#184545)
2026-04-23 rmacnak@google.com [fuchsia] Ask for only VMEX, not ambient-replace-as-executable. (flutter/flutter#185099)
2026-04-23 47866232+chunhtai@users.noreply.github.com Unify SemanticUpdateBuilder API for web and non-web (flutter/flutter#185433)
2026-04-22 68919043+Istiak-Ahmed78@users.noreply.github.com Fix ImageInfo.isCloneOf tests by moving async work to setUp (flutter/flutter#185254)
2026-04-22 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from UdpQnaP5eSaDZd3-i... to j3UCWZhWx7zSl9Asy... (flutter/flutter#185438)
2026-04-22 30870216+gaaclarke@users.noreply.github.com Adds script to run malioc locally (flutter/flutter#185371)
2026-04-22 47866232+chunhtai@users.noreply.github.com Add await mechanism to setClipboard in android_semantics_integration test (flutter/flutter#185428)
2026-04-22 43054281+camsim99@users.noreply.github.com Add ability to pass flags to `et run` (flutter/flutter#185109)
2026-04-22 47866232+chunhtai@users.noreply.github.com Add more guidelines for code review bot (flutter/flutter#185367)
2026-04-22 danny@tuppeny.com Roll pub packages (flutter/flutter#185274)
2026-04-22 saurabhmirajkar000@gmail.com Fix incorrect scale parameter reference in Image constructor docs (flutter/flutter#185403)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
...
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
@engine-flutter-autoroll
Roll Flutter from 5e4f16931847 to aeb96234de86 (42 revisions) (flutte…
5c71af3 
…r#11576)

Roll Flutter from 5e4f16931847 to aeb96234de86 (42 revisions)

flutter/flutter@5e4f169...aeb9623

2026-04-24 robert.ancell@canonical.com Fix leak on error case (flutter/flutter#185516)
2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from 04c6c369cfd0 to 1c9b0b9141e9 (2 revisions) (flutter/flutter#185529)
2026-04-24 engine-flutter-autoroll@skia.org Roll Dart SDK from f386b11262f6 to c26627715892 (1 revision) (flutter/flutter#185526)
2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from 290a056fcd0e to 04c6c369cfd0 (2 revisions) (flutter/flutter#185525)
2026-04-24 chris@bracken.jp [ios] Extract SplashScreenManager from FlutterViewController (flutter/flutter#185405)
2026-04-24 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from j3UCWZhWx7zSl9Asy... to 9fPnyEo9PaNdXtasl... (flutter/flutter#185523)
2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from 4c8bedd3c932 to 290a056fcd0e (1 revision) (flutter/flutter#185518)
2026-04-24 engine-flutter-autoroll@skia.org Roll Dart SDK from 70665fc3fd2e to f386b11262f6 (2 revisions) (flutter/flutter#185512)
2026-04-24 97480502+b-luk@users.noreply.github.com Handle hairline strokes in UberSDF (flutter/flutter#184895)
2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from ea20c73ac72c to 4c8bedd3c932 (3 revisions) (flutter/flutter#185509)
2026-04-24 58529443+srujzs@users.noreply.github.com Use relative path for reloadedSourcesUri and reloaded modules (flutter/flutter#184598)
2026-04-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Run all flutter/flutter macOS tests using Xcode 26 and iOS 26 simulator (#185431)" (flutter/flutter#185513)
2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from e8d00d634c22 to ea20c73ac72c (8 revisions) (flutter/flutter#185500)
2026-04-23 okorohelijah@google.com Run all flutter/flutter macOS tests using Xcode 26 and iOS 26 simulator (flutter/flutter#185431)
2026-04-23 rmolivares@renzo-olivares.dev update team-text-input pr triage link to filter out "waiting for response" label (flutter/flutter#185499)
2026-04-23 jason-simmons@users.noreply.github.com Check for overflow when computing the pixel buffer size for an animated PNG frame (flutter/flutter#185442)
2026-04-23 reinar@crypt.ws Impeller: Recreate Vulkan transients on surface size change (flutter/flutter#185122)
2026-04-23 matt.kosarek@canonical.com Updating the windowing API for sized to content regular and dialog windows + removing the decorated flag (flutter/flutter#184977)
2026-04-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 634991935e9a to 70665fc3fd2e (2 revisions) (flutter/flutter#185488)
2026-04-23 louisehsu@google.com Updating ios triage link (flutter/flutter#185437)
2026-04-23 34871572+gmackall@users.noreply.github.com Revert "Preprovision Android NDK for flavored builds and reuse matchi… (flutter/flutter#185439)
2026-04-23 1961493+harryterkelsen@users.noreply.github.com fix(web): Fix LateInitializationError in CkSurface and SkwasmSurface (flutter/flutter#185116)
2026-04-23 1961493+harryterkelsen@users.noreply.github.com [web] Implement stepped image downscaling for CanvasKit and Skwasm (flutter/flutter#184741)
2026-04-23 30870216+gaaclarke@users.noreply.github.com Made wide_gamut_macos only run on arm64 machines (flutter/flutter#185486)
2026-04-23 chris@bracken.jp [ios] Update documentation for FlutterAppDelegate.pluginRegistrant (flutter/flutter#185201)
2026-04-23 engine-flutter-autoroll@skia.org Roll Dart SDK from bdf48933f3cf to 634991935e9a (1 revision) (flutter/flutter#185462)
2026-04-23 engine-flutter-autoroll@skia.org Roll Skia from 5fe6162546b1 to e8d00d634c22 (3 revisions) (flutter/flutter#185459)
2026-04-23 engine-flutter-autoroll@skia.org Roll Skia from 0049c5d91b08 to 5fe6162546b1 (1 revision) (flutter/flutter#185455)
2026-04-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 9648f446f131 to bdf48933f3cf (19 revisions) (flutter/flutter#185451)
2026-04-23 engine-flutter-autoroll@skia.org Roll Skia from 11640d1cbc5c to 0049c5d91b08 (11 revisions) (flutter/flutter#185453)
2026-04-23 ishaquehassan@gmail.com Add disposal guidance to CurvedAnimation and CurveTween docs (flutter/flutter#184569)
2026-04-23 ishaquehassan@gmail.com Add `clipBehavior` parameter to `AnimatedCrossFade` (flutter/flutter#184545)
2026-04-23 rmacnak@google.com [fuchsia] Ask for only VMEX, not ambient-replace-as-executable. (flutter/flutter#185099)
2026-04-23 47866232+chunhtai@users.noreply.github.com Unify SemanticUpdateBuilder API for web and non-web (flutter/flutter#185433)
2026-04-22 68919043+Istiak-Ahmed78@users.noreply.github.com Fix ImageInfo.isCloneOf tests by moving async work to setUp (flutter/flutter#185254)
2026-04-22 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from UdpQnaP5eSaDZd3-i... to j3UCWZhWx7zSl9Asy... (flutter/flutter#185438)
2026-04-22 30870216+gaaclarke@users.noreply.github.com Adds script to run malioc locally (flutter/flutter#185371)
2026-04-22 47866232+chunhtai@users.noreply.github.com Add await mechanism to setClipboard in android_semantics_integration test (flutter/flutter#185428)
2026-04-22 43054281+camsim99@users.noreply.github.com Add ability to pass flags to `et run` (flutter/flutter#185109)
2026-04-22 47866232+chunhtai@users.noreply.github.com Add more guidelines for code review bot (flutter/flutter#185367)
2026-04-22 danny@tuppeny.com Roll pub packages (flutter/flutter#185274)
2026-04-22 saurabhmirajkar000@gmail.com Fix incorrect scale parameter reference in Image constructor docs (flutter/flutter#185403)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@b-luk b-luk b-luk approved these changes
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CICD Run CI/CD e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gaaclarke @b-luk