Skip to content

Conversation

@not-matthias
Copy link
Member

@not-matthias not-matthias commented Nov 14, 2025

Fixes #37

Changes in this PR:

  • Delete raw_results after we parse it to avoid out of disk errors
  • The templater now returns a CodspeedContext struct which stores all the temporary modifications that should be reverted afterwards.
    • This required a refactor of the patcher to save the previous file paths and package names. This was needed because you could define a global flag in another package that would result in a build error (happened in the hugo project).
  • Switched to mialloc because we're spending a lot of time allocating memory

@not-matthias not-matthias force-pushed the cod-1694-allow-configuring-cleanup-of-temporary-files branch 2 times, most recently from 20e1913 to ec303fc Compare November 14, 2025 16:45
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 14, 2025

CodSpeed Performance Report

Merging #38 will degrade performances by 15.35%

Comparing cod-1694-allow-configuring-cleanup-of-temporary-files (c5f02fb) with main (0b914bf)

Summary

⚡ 1 improvement
❌ 5 regressions
✅ 34 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
bench_collect_results[1000000, 10] 211.8 ms 247.7 ms -14.46%
bench_collect_results[1000000, 25] 447.2 ms 523.3 ms -14.55%
bench_collect_results[1000000, 5] 181.4 ms 213.2 ms -14.9%
bench_collect_results[500000, 10] 104.3 ms 123.2 ms -15.35%
bench_collect_results[500000, 25] 225.4 ms 256.1 ms -12%
bench_go_runner 59.7 s 22.4 s ×2.7

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses out-of-disk-space issues by optimizing temporary directory usage and implementing proper cleanup mechanisms. The changes introduce a shared temporary directory across packages and automatic reversion of file modifications.

  • Refactored Templater to use a single shared temporary directory via OnceCell instead of creating one per package
  • Introduced CodspeedContext struct to encapsulate package-specific setup with cleanup tracking via a Patcher that reverts changes on drop
  • Added cleanup of raw results directory after parsing to save disk space

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
go-runner/tests/utils.rs Updated test utilities to use new Templater API with CodspeedContext
go-runner/src/results/raw_result.rs Changed glob pattern to expect raw results directly in provided folder path
go-runner/src/lib.rs Added raw_results subdirectory handling, cleanup logic, and empty results validation
go-runner/src/builder/templater.rs Major refactor introducing CodspeedContext and shared temp directory via OnceCell
go-runner/src/builder/patcher.rs Extracted Patcher struct with state tracking and automatic reversion via Drop trait
go-runner/Cargo.toml Added once_cell dependency for lazy initialization
Cargo.lock Updated lockfile with once_cell dependency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@not-matthias not-matthias force-pushed the cod-1694-allow-configuring-cleanup-of-temporary-files branch from ec303fc to 95f922e Compare November 14, 2025 17:20
@not-matthias not-matthias force-pushed the cod-1694-allow-configuring-cleanup-of-temporary-files branch from c2eb2fe to 4ccc1fb Compare November 14, 2025 18:09
Copy link

@GuillaumeLagrange GuillaumeLagrange left a comment

Choose a reason for hiding this comment

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

small comments + the changes we discussed to make use of git to revert rather than manually undoing the patches

@not-matthias not-matthias force-pushed the cod-1694-allow-configuring-cleanup-of-temporary-files branch from 4e07ea9 to 176cd46 Compare November 17, 2025 11:03
@not-matthias not-matthias force-pushed the cod-1694-allow-configuring-cleanup-of-temporary-files branch 2 times, most recently from 97cc75c to efb0b12 Compare November 17, 2025 11:39
Copy link

@GuillaumeLagrange GuillaumeLagrange left a comment

Choose a reason for hiding this comment

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

olgtm, almost there!

@not-matthias not-matthias force-pushed the cod-1694-allow-configuring-cleanup-of-temporary-files branch from efb0b12 to c5f02fb Compare November 17, 2025 13:26
@not-matthias not-matthias merged commit c5f02fb into main Nov 17, 2025
15 of 16 checks passed
@not-matthias not-matthias deleted the cod-1694-allow-configuring-cleanup-of-temporary-files branch November 17, 2025 14:10
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.

Allow configuring cleanup of temporary files

4 participants