Skip to content

Optimize build_instrumented job#6954

Merged
grzesiek2010 merged 13 commits intogetodk:masterfrom
seadowg:test-build-memory
Nov 4, 2025
Merged

Optimize build_instrumented job#6954
grzesiek2010 merged 13 commits intogetodk:masterfrom
seadowg:test-build-memory

Conversation

@seadowg
Copy link
Member

@seadowg seadowg commented Nov 3, 2025

This fixes the build_instrumented step by giving it a larger resource and better suited memory config. I've also done some general touch ups on caching etc and added manual approval to PR CI runs. I'm hoping all of this increases CI run time and stability without massively increasing costs.

@seadowg seadowg changed the title Optimize build_instrumented job Optimize build_instrumented job Nov 3, 2025
@seadowg seadowg force-pushed the test-build-memory branch 2 times, most recently from 8399748 to bd27a16 Compare November 3, 2025 14:05
paths:
- ~/.gradle/caches/modules-2/files-2.1
key: compile-deps-{{ checksum "deps.txt" }}
- ~/work
Copy link
Member Author

@seadowg seadowg Nov 4, 2025

Choose a reason for hiding this comment

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

Interestingly, saving to cache is much faster than persisting to workspace for large sets of files. Caching makes more sense here as well as we don't need the files we're saving here: they just help speed subsequent jobs in the worklow up.

root: ~/work
paths:
- .
- collect_app/build/outputs/apk
Copy link
Member Author

Choose a reason for hiding this comment

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

We still need to save the APK so that it can be definitely available to the Test Lab jobs.

./check-size.sh 23117869
else
./check-size.sh 13785543
./check-size.sh 13841204
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why this increase? It's a very small bump though.

commit:
jobs:
- compile
- hold_pr:
Copy link
Member Author

Choose a reason for hiding this comment

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

I often find that I might be committing/pushing quickly and that context, CI will be constantly starting and cancelling workflow runs. Add a manual approval step means we only end up running jobs when we actually care about the result. I'd like to try this, but definitely let me know if you feel like it's getting annoying @grzesiek2010.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with your rationale, but isn’t it possible to differentiate between draft and ready-for-review PRs and trigger jobs only for those that are ready for review?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea! It looks like this might be possible with the new Github triggers, but I'm not sure that we'd be able to have it always run for non-drafts and optionally for drafts (you still probably want CI on request for drafts). We could always just use the GitHub API (probably through gh).

I think that's probably a future improvement though: I think we either go with the basic version as-is or remove the hold_pr check for now.

@seadowg seadowg marked this pull request as ready for review November 4, 2025 11:24
@seadowg seadowg requested a review from grzesiek2010 November 4, 2025 11:24
@seadowg seadowg added the high priority Should be looked at before other PRs/issues label Nov 4, 2025
branches:
only:
- master
- compile:
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to have two jobs with the same name (compile)? Does it work or one replaces the other?

Copy link
Member Author

Choose a reason for hiding this comment

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

They both reference the compile job definition - this isn't declaring the job. One runs on master only and the other only on non-master.

@seadowg seadowg requested a review from grzesiek2010 November 4, 2025 13:53
@grzesiek2010 grzesiek2010 merged commit b746986 into getodk:master Nov 4, 2025
7 checks passed
@seadowg seadowg deleted the test-build-memory branch November 4, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

high priority Should be looked at before other PRs/issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants