Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix caching on databroker build #29

Merged

Conversation

SebastianSchildt
Copy link
Contributor

@SebastianSchildt SebastianSchildt commented May 9, 2024

There was a typo that prevents full caching of build artifacts for databroker build (CLI was ok already). Fixed now.

Example build with no cache (so probably also the build here, as changing cache contents invalidates cache)

Screenshot 2024-05-09 at 13 43 53

Subsequent build satisfying cache requirements

Screenshot 2024-05-09 at 13 47 12

(see here https://github.com/boschglobal/kuksa-databroker/actions?query=event%3Apull_request+branch%3Atest%2Fcaching for various test builds)

I also checked where time is mainly used now

  • 20+ sec is always used pulling the cross images (crossb uilds using docker). the cross cargo package is cached and not reinstalled but the docker images are not. I do not intend to look into this/fix this in this PR. Just a hint, if somebody feels bored. I am however not sure if there is even a feasible solution as those images are around 2 GB each and I think there are strict limitations on how much cache can be used before things get evicted)
  • Another large chunk is basically the "linking" so I think, this has to be accepted.

@SebastianSchildt SebastianSchildt requested a review from argerus May 9, 2024 05:08
@argerus

This comment was marked as duplicate.

Copy link
Contributor

@argerus argerus left a comment

Choose a reason for hiding this comment

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

This seems to fix caching for the build step.

The overall build is still slower than it was since caching of the unit test step was also broken in the original PR.

Screenshot from 2024-05-10 14-41-25

I suppose always having the target dir under target/ would make it easier to get this right, i.e. using --target-dir "target/cross-$target" in order to work around the issue with building for different systems under cross.

Signed-off-by: Sebastian Schildt <[email protected]>
@SebastianSchildt SebastianSchildt force-pushed the fix/databroker-build-caching branch from 2839451 to 712d533 Compare May 10, 2024 13:20
@SebastianSchildt
Copy link
Contributor Author

Fixed. I choose to fix the cache definition in workflow instead of changing dirs in script.
(I was thinking if somebody during dev is using plain "cargo build/run" and also in between places with the build scripts, the structure inside the default target folder might seem confusing. Also... easier/faster to test for me just now)

@SebastianSchildt SebastianSchildt requested a review from argerus May 10, 2024 13:35
@SebastianSchildt SebastianSchildt merged commit 06250f4 into eclipse-kuksa:main May 10, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants