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

Reduce time for workflow/Build #1709

Merged
merged 8 commits into from
Dec 24, 2021
Merged

Reduce time for workflow/Build #1709

merged 8 commits into from
Dec 24, 2021

Conversation

NiKiZe
Copy link
Collaborator

@NiKiZe NiKiZe commented Dec 23, 2021

  • Use cache for .pio path, this keeps buildtime down when unchanged.
  • Use dynamic matrix to run jobs in parallel
    • These jobs are generated from location of platformio.ini files
    • All jobs runs on their own, some duplication of setup
    • Status is reported for each job
    • One final job that requires all other jobs to succeed

@crankyoldgit
Copy link
Owner

Good luck. 🤞

@NiKiZe NiKiZe changed the title Test reduce build time Reduce time for workflow/Build Dec 23, 2021
@NiKiZe NiKiZe marked this pull request as ready for review December 23, 2021 15:50
@NiKiZe NiKiZe requested a review from crankyoldgit December 23, 2021 15:50
@NiKiZe
Copy link
Collaborator Author

NiKiZe commented Dec 23, 2021

Normal runtime was around 30 minutes, we are down to 2-3 minutes as long as there is no code change.
Might be good to test this with code change to see how it responds, especially so that it don't fail. but also check that the cache is updated.

NiKiZe added a commit that referenced this pull request Dec 23, 2021
@NiKiZe
Copy link
Collaborator Author

NiKiZe commented Dec 23, 2021

Tested in #1711 Total runtime of Builds work was 15m 35s, the longest was IRrecvDumpV3
New build cache was not used.
And messages like Unable to reserve cache with key Linux-, another job may be creating this cache. are seen when they should be created.

I tried to look into having a common setup step and then run jobs, but did not find anything really promising.

@NiKiZe NiKiZe force-pushed the test_build_library_json branch from c87b67f to 94d1357 Compare December 23, 2021 18:27
@NiKiZe
Copy link
Collaborator Author

NiKiZe commented Dec 23, 2021

After modifying cache key for pio build data and create one per matrix item it now builds fine and also reuses cached for later started builds.

@NiKiZe NiKiZe self-assigned this Dec 23, 2021
Copy link
Owner

@crankyoldgit crankyoldgit left a comment

Choose a reason for hiding this comment

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

I'd be lying if I said I understood it fully. But the results are awesome.

Approved!

@NiKiZe NiKiZe merged commit 709af24 into master Dec 24, 2021
@NiKiZe NiKiZe deleted the test_build_library_json branch December 24, 2021 07:09
Comment on lines +54 to +57
key: pio-${{ runner.os }}-${{ matrix.project }}
restore-keys: |
pio-${{ runner.os }}-
pio-
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Might need to revisit this to get new caches created correctly. Now Sony was changed and as other files change as well we might lose cache hits.

Copy link
Owner

@crankyoldgit crankyoldgit Dec 30, 2021

Choose a reason for hiding this comment

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

Just for the record, I completely have no clue what you're saying. :-P Well, I understand what you're saying, but I don't know what it technically means. i.e. I have no clue how any of your science caching magic works. I say you're a witch. Witch! Burn him! 🔥

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This part extracts the .pio directory from a tar archive before build so that cached builds can be used.
As times goes on more and more of the build cache will become out of date.
Since the key already matches perfectly new archives are not created.

Nothing will (probably) break, but builds will get slower again.
If something breaks just pull out that cache from the yml.

I do intend to keep an eye on this and create some kind of fix.

magic is just technology we don't understand

I don't know either, I just modified the existing yml that you created. - now which is the witch? :P

Copy link
Owner

Choose a reason for hiding this comment

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

I do intend to keep an eye on this and create some kind of fix.

So if I understand you correctly, we are referencing a point-in-time object cache, which will grow outdated with every change.

Can we automate creating the cache at all? Say, a weekly or daily etc tasks with what ever the master branch is at?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the key is only a string.
We could use a key with the commit hash, but that would generate new caches on each commit.
But adding year and week number is probably a really good balance between not creating it every time causing many files in cache and not having it to outdated.

Right now we don't have much data, my thinking is to do the revisit when we see this causing an impact.
In worst case we should see build times of 12-15 minutes instead of the 2-3 minutes we have now. (but that is still better than the 30 minutes we had before)

Copy link
Collaborator Author

@NiKiZe NiKiZe Dec 30, 2021

Choose a reason for hiding this comment

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

There is key: and restore-keys
if key matches it is used and no new cache is created.
On non key match, restore-keys is searched for alternative, and a new cache is created.

by adding more conditions to the main key, new caches will be created.
Max 10GB of caches in total will be kept.

https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows#input-parameters-for-the-cache-action

Maybe better example: https://github.com/actions/cache#creating-a-cache-key

Copy link
Owner

Choose a reason for hiding this comment

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

sounds good to me. I'll leave it with you. ;-)

crankyoldgit added a commit that referenced this pull request Dec 31, 2021
_v2.8.1 (20220101)_

**[Bug Fixes]**
- Arduino ESP32 Core v2.0.2+ crashes due to our timer hack. (#1715 #1715)
- SONY: Fix old Sony CD-Player Remote (12 Bit) (#1714)

**[Features]**
- Add tool to convert protocol & code to raw timing info. (#1708 #1707 #1703)
- Add basic support for COOLIX48 protocol. (#1697 #1694)
- MITSUBISHI_AC: Added support for i-SAVE mode. (#1666)
- TOSHIBA_AC: Add Filter setting support. aka. Pure. (#1693 #1692)
- Airton: Add detailed A/C support. (#1688 #1670)

**[Misc]**
- Add a structured library version number. (#1717)
- Workflows Split UnitTests (#1712)
- Reduce time for workflow/Build (#1709)
- Fix some compiler & linter warnings (#1699 #1700)
- Fujitsu: Update supported A/C models (#1690 #1689 #1702 #1701)
- Remove extra `const` qualifier for char pointer (#1704)
- TCL: Update supported devices. (#1698)
- ESP32-C3: Work around for some C3 specific compiler issues. (#1696 #1695)
@crankyoldgit crankyoldgit mentioned this pull request Dec 31, 2021
crankyoldgit added a commit that referenced this pull request Jan 1, 2022
## _v2.8.1 (20220101)_

**[Bug Fixes]**
- Arduino ESP32 Core v2.0.2+ crashes due to our timer hack. (#1715 #1713 )
- SONY: Fix old Sony CD-Player Remote (12 Bit) (#1714)

**[Features]**
- Add tool to convert protocol & code to raw timing info. (#1708 #1707 #1703)
- Add basic support for COOLIX48 protocol. (#1697 #1694)
- MITSUBISHI_AC: Added support for i-SAVE mode. (#1666)
- TOSHIBA_AC: Add Filter setting support. aka. Pure. (#1693 #1692)
- Airton: Add detailed A/C support. (#1688 #1670)

**[Misc]**
- Add a structured library version number. (#1717)
- Workflows Split UnitTests (#1712)
- Reduce time for workflow/Build (#1709)
- Fix some compiler & linter warnings (#1699 #1700)
- Fujitsu: Update supported A/C models (#1690 #1689 #1702 #1701)
- Remove extra `const` qualifier for char pointer (#1704)
- TCL: Update supported devices. (#1698)
- ESP32-C3: Work around for some C3 specific compiler issues. (#1696 #1695)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants