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

feat: purge build cache #2033

Merged
merged 14 commits into from
Feb 13, 2023
Merged

feat: purge build cache #2033

merged 14 commits into from
Feb 13, 2023

Conversation

Bikappa
Copy link
Contributor

@Bikappa Bikappa commented Jan 10, 2023

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)

What kind of change does this PR introduce?

The build files saved under temporary folders gets cleaned regularly if not used.

What is the current behavior?

Unused files remain stale in temporary folders until the os decides to eventually remove them

What is the new behavior?

After an amount of compilations the built files are deleted if they were last used before a period of time.
The number compilations before the purge is triggered and the maximum age of the files are configurable with the following environment variables:

  • ARDUINO_BUILD_CACHE_COMPILATIONS_BEFORE_PURGE (0 signifies never purge, default to 10)
  • ARDUINO_BUILD_CACHE_TTL (default to 30 days)

This does only apply the default, temporary, build path. It does not purge files under user-specified locations, outside of the default one.

The default directory structures is also changed from:

arduino
├── core-cache
│   ├── core-x.a
│   ├── core-y.a
│   └── core-z.a
├── sketch-1
├── sketch-2
└── sketch-3

to

arduino
├── cores
│   ├── x
│   │   └── core.a
│   ├── y
│   │   └── core.a
│   └── z
│       └── core.a
└── sketches
    ├── 1
    ├── 2
    └── 3

Does this PR introduce a breaking change, and is titled accordingly?

Other information

Fixes #2029

@Bikappa Bikappa force-pushed the feat/purge-build-cache branch 5 times, most recently from a7060fb to 2d5c5f0 Compare January 11, 2023 16:01
@codecov
Copy link

codecov bot commented Jan 11, 2023

Codecov Report

Base: 36.68% // Head: 36.74% // Increases project coverage by +0.06% 🎉

Coverage data is based on head (4c64cb5) compared to base (396718f).
Patch coverage: 37.50% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2033      +/-   ##
==========================================
+ Coverage   36.68%   36.74%   +0.06%     
==========================================
  Files         228      229       +1     
  Lines       19385    19456      +71     
==========================================
+ Hits         7111     7150      +39     
- Misses      11442    11472      +30     
- Partials      832      834       +2     
Flag Coverage Δ
unit 36.74% <37.50%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
commands/compile/compile.go 0.00% <0.00%> (ø)
buildcache/build_cache.go 46.34% <46.34%> (ø)
legacy/builder/phases/core_builder.go 58.25% <58.33%> (-0.52%) ⬇️
arduino/sketch/sketch.go 63.79% <100.00%> (ø)
configuration/defaults.go 100.00% <100.00%> (ø)
...egacy/builder/add_additional_entries_to_context.go 62.50% <100.00%> (ø)
commands/lib/search.go 92.10% <0.00%> (+3.94%) ⬆️
arduino/monitor/monitor.go 47.36% <0.00%> (+6.31%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Bikappa Bikappa changed the title Feat/purge build cache feat: purge build cache Jan 11, 2023
@Bikappa Bikappa marked this pull request as ready for review January 12, 2023 09:14
@Bikappa Bikappa self-assigned this Jan 12, 2023
Copy link
Member

@cmaglie cmaglie left a comment

Choose a reason for hiding this comment

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

The go-paths can be exploited much more here, I didn't comment everywhere, but you surely got the pattern :-)

buildcache/build_cache_test.go Outdated Show resolved Hide resolved
buildcache/build_cache_test.go Outdated Show resolved Hide resolved
buildcache/build_cache_test.go Outdated Show resolved Hide resolved
buildcache/build_cache_test.go Outdated Show resolved Hide resolved
buildcache/build_cache_test.go Outdated Show resolved Hide resolved
@Bikappa Bikappa force-pushed the feat/purge-build-cache branch 2 times, most recently from b766637 to df1ec17 Compare January 13, 2023 17:02
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Jan 15, 2023
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

UPDATE: resolved by 7082f17

Please add the new `build_cache.compilations_before_purge` and `build_cache.ttl` configuration keys to the documentation:

https://github.com/arduino/arduino-cli/blob/feat/purge-build-cache/docs/configuration.md

arduino/sketch/sketch.go Outdated Show resolved Hide resolved
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

The .last-used file is not being written to the core cache. This causes the core to always be considered older than TTL.

@Bikappa Bikappa force-pushed the feat/purge-build-cache branch 2 times, most recently from f5a4c7d to 7082f17 Compare January 16, 2023 08:17
@Bikappa
Copy link
Contributor Author

Bikappa commented Jan 16, 2023

@cmaglie @per1234 I went through your comments 👍 and updated accordingly

buildcache/directory_cache.go Outdated Show resolved Hide resolved
commands/compile/compile.go Outdated Show resolved Hide resolved
buildcache/directory_cache.go Outdated Show resolved Hide resolved
buildcache/build_cache_test.go Outdated Show resolved Hide resolved
buildcache/directory_cache.go Outdated Show resolved Hide resolved
internal/integrationtest/compile_1/compile_test.go Outdated Show resolved Hide resolved
legacy/builder/phases/core_builder.go Outdated Show resolved Hide resolved
buildcache/directory_cache.go Outdated Show resolved Hide resolved
buildcache/build_cache.go Outdated Show resolved Hide resolved
internal/integrationtest/compile_1/compile_test.go Outdated Show resolved Hide resolved
@Bikappa Bikappa force-pushed the feat/purge-build-cache branch 2 times, most recently from 78ede0d to 6afc55a Compare January 16, 2023 17:23
legacy/builder/phases/core_builder.go Outdated Show resolved Hide resolved
legacy/builder/phases/core_builder.go Outdated Show resolved Hide resolved
legacy/builder/phases/core_builder.go Outdated Show resolved Hide resolved
buildcache/directory_cache.go Outdated Show resolved Hide resolved
legacy/builder/phases/core_builder.go Outdated Show resolved Hide resolved
@Bikappa Bikappa marked this pull request as draft January 17, 2023 08:06
@Bikappa Bikappa force-pushed the feat/purge-build-cache branch 3 times, most recently from 80edb72 to 5d44cb3 Compare January 17, 2023 18:20
@Bikappa Bikappa marked this pull request as ready for review January 30, 2023 13:22
buildcache/build_cache.go Outdated Show resolved Hide resolved
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

This is a very nice system. Thanks Luca!

@Bikappa Bikappa force-pushed the feat/purge-build-cache branch 3 times, most recently from 3293e09 to 5296056 Compare February 7, 2023 07:21
@Bikappa Bikappa requested a review from cmaglie February 13, 2023 13:41
@Bikappa Bikappa merged commit 53a6f25 into master Feb 13, 2023
@Bikappa Bikappa deleted the feat/purge-build-cache branch February 13, 2023 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete unused compilation caches
3 participants