Skip to content

Reduce build time#671

Merged
alexander-yevsyukov merged 38 commits into
masterfrom
measure-build-time
Aug 28, 2021
Merged

Reduce build time#671
alexander-yevsyukov merged 38 commits into
masterfrom
measure-build-time

Conversation

@alexander-yevsyukov

@alexander-yevsyukov alexander-yevsyukov commented Aug 27, 2021

Copy link
Copy Markdown
Contributor

This PR partially addresses #670. On local tests, the build time reduced from 18 to 13 minutes. It's not much, but still saves some life...

The build time was reduced by reducing the number of test Gradle projects created and compiled. So, what we did on @BeforeEach, we do now @BeforeAll.

Another improvement of this PR is fixing the way we treat temporary directories created by tests. They were not deleted, and accumulated under the root temporary directory. The TempDir instances now are created under the io.spine.testing directory, which is deleted on JVM exit. This way it's easy to see «кто насрал», even if the cleanup fails.

More work on making our treatment on temporary disk space needs to be done (SpineEventEngine/config#240).

@alexander-yevsyukov alexander-yevsyukov self-assigned this Aug 27, 2021
@codecov

codecov Bot commented Aug 27, 2021

Copy link
Copy Markdown

Codecov Report

Merging #671 (dcbcaa9) into master (1be70dc) will decrease coverage by 0.06%.
The diff coverage is 61.70%.

@@             Coverage Diff              @@
##             master     #671      +/-   ##
============================================
- Coverage     77.41%   77.34%   -0.07%     
- Complexity     3294     3301       +7     
============================================
  Files           552      553       +1     
  Lines         12479    12507      +28     
  Branches        752      753       +1     
============================================
+ Hits           9660     9673      +13     
- Misses         2531     2545      +14     
- Partials        288      289       +1     

... trying to save on creating temp. projects.
@alexander-yevsyukov alexander-yevsyukov marked this pull request as ready for review August 28, 2021 00:33
@alexander-yevsyukov alexander-yevsyukov requested review from a team and yuri-sergiichuk August 28, 2021 00:34

@yuri-sergiichuk yuri-sergiichuk 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.

@alexander-yevsyukov overall LGTM. Please consider a couple of suggestions and comments below prior to merging the PR.

My only concern with using static (per test suite and not per test case) projects is possible issues with working with a non-fresh project. If that's not smth that may anyhow mess up the tests, I'm with both hands on merging the PR.

Comment thread base/src/main/java/io/spine/io/Files2.java
Comment thread testlib/src/main/java/io/spine/testing/TempDir.java Outdated
Comment thread testlib/src/main/java/io/spine/testing/TempDir.java Outdated
@alexander-yevsyukov alexander-yevsyukov merged commit bd9ab25 into master Aug 28, 2021
@alexander-yevsyukov alexander-yevsyukov deleted the measure-build-time branch August 28, 2021 17:06
alexander-yevsyukov added a commit to SpineEventEngine/config that referenced this pull request Jul 1, 2026
The `updateGitHubPages` task created the `javadoc`, `html`, and `repoTemp`
temporary directories via `LazyTempPath` directly under the system temp
directory, with cleanup performed only eagerly (`UpdateGitHubPages.cleanup()`
and `Repository.close()`). A failed build or a killed Gradle daemon leaked
them permanently. `File.deleteOnExit()` cannot help — it does not delete
non-empty directories.

Create every such directory under a shared base directory
(`<java.io.tmpdir>/io.spine.gradle.fs`) that is registered for recursive
deletion via a single JVM shutdown hook, mirroring the approach of
SpineEventEngine/base-libraries#671. The eager cleanup stays the primary mechanism;
the shutdown hook is the safety net.

Fixes #240.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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