perf(filesets): Improve filesets performance
Metadata
Current evaluation
Merged PR #1760 improves filesets migration performance by reverting path resolution to string operations and caching Path/PurePath and parent/realpath resolution.
Resolution: closed resolved
Reason: PR #1760 was merged after approval by two reviewers with no unresolved review comments. It optimizes filesets performance, with benchmarks showing reduced CPU and wall time.
Evaluated by: mr-cal-bot
No scores available.
Issue body
Benchmarks on my laptop with cpython 3.12.14:
| Implementation | `memory.peak` | relative | cpu `usage_usec` | relative | wall | relative |
| -------------- | ------------- | -------- | ---------------- | -------- | ------ | -------- |
| 059b1c0 | 75.62 MB | 1.00x | 16.84 s | 1.00x | 17.56s | 1.00x |
| 2.37.1 | 74.98 MB | 0.99x | 26.75 s | 1.59x | 29.48s | 1.68x |
| 64ec5d0 | 70.55 MB | 0.93x | 18.97 s | 1.13x | 19.96s | 1.14x |
| 70a3537 | 70.41 MB | 0.93x | 16.30 s | 0.97x | 18.57s | 1.06x |
| ceac30e | 70.68 MB | 0.93x | 14.83 s | 0.88x | 17.28s | 0.98x |
| Total PR | 70.63 MB | 0.93x | 13.44 s | 0.80x | 14.97s | 0.85x |
Because I was interested in memory, CPU usage, and wall time, I did the benchmarks on a whole run of `python -m craft_parts` for a 10-part `parts.yaml` file with directories that each contain 1000 empty files. As such, the timing differences look less extreme than what @mr-cal's [comment here](https://github.com/canonical/craft-parts/pull/1424#discussion_r4208254299) showed. I validated the same relative difference in just running a `stage`.
This might be easier to review per-commit, though I also do some fixups near the end. We do:
1. Switch `_get_resolved_relative_path` back to string operations, also caching the parent resolution and real path resolution per migration (since we expect the filesystem not to change during that time).
2. Cache the creation of both `Path` and `PurePath` objects (since they're immutable).
3. Use the caching functions from 1 and 2 in exclude sets.
4. Some minor optimizations (e.g. not creating a cwd `Path` object in a tight loop).
---
- [x] I've followed the [contribution guidelines](https://github.com/canonical/craft-parts/blob/main/CONTRIBUTING.md).
- [x] I've signed the [CLA](http://www.ubuntu.com/legal/contributors/).
- [x] I've successfully run `make lint && make test`.
- [x] I've added or updated any relevant documentation.
- [ ] In documents I changed, I [added a meta description](https://canonical-starflow.readthedocs-hosted.com/how-to/add-a-page-meta-description/) if one was missing.
- [ ] I've updated the relevant release notes.
Evaluation history
| Date | Model | Scores | Action | Summary |
|---|---|---|---|---|
| ~deepseek/deepseek-pro-latest | — | closed resolved | Merged PR #1760 improves filesets migration performance by reverting path resolution to string operations and caching Path/PurePath and parent/realpath resolution. | |
| ~deepseek/deepseek-pro-latest |
Impact:
55
Quick Win:
23
Complexity:
50
Confidence:
82
Actionability:
85
|
needs review | Improves filesets migration performance via string-based path resolution and caching of Path/PurePath and parent/realpath resolution; approved by 2 reviewers, CI partially pending. | |
| ~deepseek/deepseek-pro-latest |
Impact:
55
Quick Win:
20
Complexity:
55
Confidence:
70
Actionability:
80
|
needs review | Improves filesets migration performance by reverting path resolution to string ops and caching Path/PurePath and parent/realpath resolution; approved, CI partially pending. | |
| ~deepseek/deepseek-pro-latest |
Impact:
50
Quick Win:
20
Complexity:
50
Confidence:
70
Actionability:
80
|
needs review | Reverts fileset path resolution to string ops and caches Path/PurePath and parent/realpath resolution; ~15% wall-time and ~7% memory gains. Approved by 2 reviewers; no unresolved comments; CI pending. | |
| ~deepseek/deepseek-pro-latest |
Impact:
55
Quick Win:
15
Complexity:
55
Confidence:
70
Actionability:
60
|
needs review | Reverts fileset path resolution to string ops and caches Path/PurePath and parent/realpath resolution; ~15% wall-time and ~7% memory gains. Approved by 2 reviewers; 6 unresolved comments; some CI failures. | |
| ~deepseek/deepseek-pro-latest |
Impact:
50
Quick Win:
14
Complexity:
50
Confidence:
65
Actionability:
55
|
needs review | Reverts fileset path resolution to string ops and caches Path/PurePath and parent/realpath resolution; ~15% wall-time and ~7% memory gains. Approved by 2 reviewers; 5 unresolved comments; some unrelated CI failures. | |
| ~deepseek/deepseek-pro-latest |
Impact:
50
Quick Win:
14
Complexity:
48
Confidence:
65
Actionability:
55
|
needs review | Optimizes fileset migration: string-based path resolution, cached Path/PurePath and parent/realpath resolution; benchmarks show ~15% wall-time and ~20% CPU reduction. Approved by 2 reviewers with 5 unresolved comments; some CI failing/pending. | |
| ~deepseek/deepseek-pro-latest |
Impact:
55
Quick Win:
15
Complexity:
55
Confidence:
70
Actionability:
60
|
needs review | Optimizes fileset migration by reverting path resolution to string operations and caching Path/PurePath and parent/realpath resolution. Approved with 4 unresolved review comments; CI partially pending. | |
| ~deepseek/deepseek-pro-latest |
Impact:
55
Quick Win:
11
Complexity:
50
Confidence:
65
Actionability:
40
|
keep open | Optimizes fileset migration by reverting path resolution to string operations and caching Path/PurePath and parent/realpath resolution. Approved with 4 unresolved review comments; some CI checks failing. | |
| ~deepseek/deepseek-pro-latest |
Impact:
55
Quick Win:
11
Complexity:
50
Confidence:
65
Actionability:
40
|
keep open | Optimizes fileset migration by reverting path resolution to string operations and caching Path/PurePath and parent/realpath resolution. Approved with 4 unresolved review comments; some CI checks failing. | |
| ~deepseek/deepseek-pro-latest |
Impact:
55
Quick Win:
15
Complexity:
50
Confidence:
60
Actionability:
55
|
needs review | Improves fileset migration performance via string-based path resolution and caching Path/PurePath and parent/realpath resolution. Approved with 4 unresolved review comments; some CI checks failing. | |
| ~deepseek/deepseek-pro-latest |
Impact:
50
Quick Win:
11
Complexity:
55
Confidence:
65
Actionability:
50
|
keep open | Improves fileset migration performance by switching path resolution back to string operations and caching Path/PurePath creation; approved but has 4 unresolved review comments and some failing CI checks. | |
| ~deepseek/deepseek-pro-latest |
Impact:
55
Quick Win:
17
Complexity:
45
Confidence:
65
Actionability:
55
|
needs review | Improves filesets performance by reverting path resolution to string operations and caching Path/PurePath creation; benchmarks show ~12% CPU and ~4% wall-time gains. Approved but has 4 unresolved comments and 3 failing CI checks. | |
| ~deepseek/deepseek-pro-latest |
Impact:
60
Quick Win:
12
Complexity:
60
Confidence:
70
Actionability:
50
|
keep open | Improves filesets migration performance by reverting path resolution to string ops and caching Path/PurePath objects; approved but has 4 unresolved review comments and 3 failing CI checks. | |
| ~deepseek/deepseek-pro-latest |
Impact:
55
Quick Win:
16
Complexity:
50
Confidence:
65
Actionability:
60
|
needs review | Improves fileset migration performance by switching path resolution back to string operations and caching Path/PurePath creation and parent/realpath resolution; CI pending, two unresolved review comments. | |
| ~deepseek/deepseek-pro-latest |
Impact:
55
Quick Win:
15
Complexity:
50
Confidence:
72
Actionability:
55
|
needs review | Improves fileset migration performance by switching path resolution back to string operations and caching Path/PurePath creation and parent/realpath resolution; CI has OSV-scanner and minimum-dependencies failures. | |
| ~deepseek/deepseek-pro-latest |
Impact:
48
Quick Win:
10
Complexity:
60
Confidence:
65
Actionability:
50
|
needs review | Switches fileset path resolution to string operations with cached parent/realpath and Path/PurePath creation; applies caching to exclude sets. CI failing OSV-scanner. | |
| ~deepseek/deepseek-pro-latest |
Impact:
50
Quick Win:
8
Complexity:
60
Confidence:
70
Actionability:
40
|
keep open | Switches fileset path resolution to string operations with cached parent/realpath and Path/PurePath creation; applies caching to exclude sets. CI failing lint/files and OSV-scanner. | |
| ~deepseek/deepseek-pro-latest |
Impact:
50
Quick Win:
16
Complexity:
50
Confidence:
70
Actionability:
65
|
needs review | Optimizes _get_resolved_relative_path with string resolution and cached parent/base paths; benchmark shows ~30% CPU improvement over 2.37.1. Open, pending review; OSV-scanner failing, tests pending. | |
| ~deepseek/deepseek-pro-latest |
Impact:
50
Quick Win:
9
Complexity:
40
Confidence:
60
Actionability:
30
|
keep open | Optimizes _get_resolved_relative_path to use string resolution with cached parent/base paths; ~30% CPU improvement vs 2.37.1. Open with 1 unresolved review comment; OSV-scanner failing, most tests pending. | |
| ~deepseek/deepseek-pro-latest |
Impact:
40
Quick Win:
12
Complexity:
40
Confidence:
60
Actionability:
50
|
needs review | Optimizes _get_resolved_relative_path to use string resolution with cached parent/base paths; ~30% CPU improvement vs 2.37.1. Open with 1 unresolved review comment; OSV-scanner failing, most tests pending. |
Update history
| Date | Change |
|---|---|
| merged | |
| updated | |
| updated | |
| updated | |
| updated | |
| updated | |
| updated | |
| updated | |
| updated | |
| updated | |
| updated | |
| updated | |
| updated | |
| updated | |
| created |
Related work
-
Caused By:
canonical/craft-parts#1424
(confidence 75%)
The pathlib refactor in #1424 introduced the filesets performance regression that this PR addresses.
Related issues
No related issues found above the similarity threshold.