Skip to content

perf: reuse Conda environment metadata across refreshes (Fixes #476) - #487

Merged
Karthik Nadig (karthiknadig) merged 1 commit into
mainfrom
perf/issue-476
Jul 25, 2026
Merged

perf: reuse Conda environment metadata across refreshes (Fixes #476)#487
Karthik Nadig (karthiknadig) merged 1 commit into
mainfrom
perf/issue-476

Conversation

@karthiknadig

Copy link
Copy Markdown
Member

Summary

  • cache parsed Conda environment details by conda-meta directory and history fingerprints
  • share the self-invalidating cache with transient JSON-RPC refresh locator graphs
  • rebuild reported environment and manager state each refresh while pruning removed prefixes
  • cover cross-refresh reuse and history-based invalidation

Validation

  • cargo test -p pet-conda
  • cargo test -p pet sync_refresh_locator_state
  • .\scripts\rust-precommit.ps1

Fixes #476

Cache parsed Conda environment details by metadata fingerprints and share the cache with transient refresh locator graphs while rebuilding discovery outputs each refresh.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@karthiknadig
Karthik Nadig (karthiknadig) marked this pull request as ready for review July 24, 2026 21:15
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

Test Coverage Report (Linux)

Metric Value
Current Coverage 80.3%
Base Branch Coverage 80.3%
Delta 0% ➖

Coverage unchanged.

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

Performance Report (Linux) ➖

Metric PR (P50) PR (P95) Baseline (P50) Delta Change
Server Startup 1ms 1ms 0ms 1ms N/A%
Full Refresh 53ms 274ms 48ms 5ms 10.0%

Results based on 10 iterations. P50 = median, P95 = 95th percentile.


Legend
  • 🚀 Significant speedup (>100ms faster)
  • ✅ Faster than baseline
  • ➖ No significant change
  • 🔺 Slower than baseline (>100ms)
  • ⚠️ Significant slowdown (>500ms)

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

Performance Report (Windows) ➖

Metric PR (P50) PR (P95) Baseline (P50) Delta Change
Server Startup 9ms 11ms 9ms 0ms 0%
Full Refresh 132ms 2143ms 129ms 3ms 2.3%

Results based on 10 iterations. P50 = median, P95 = 95th percentile.


Legend
  • 🚀 Significant speedup (>100ms faster)
  • ✅ Faster than baseline
  • ➖ No significant change
  • 🔺 Slower than baseline (>100ms)
  • ⚠️ Significant slowdown (>500ms)

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

Performance Report (macOS)

Metric PR (P50) PR (P95) Baseline (P50) Delta
Server Startup 83ms 7783ms 82ms 1ms
Full Refresh 140ms 30278ms 137ms 3ms

Results based on 10 iterations. P50 = median, P95 = 95th percentile.


Legend
  • 🚀 Significant speedup (>100ms faster)
  • ✅ Faster than baseline
  • ➖ No significant change
  • 🔺 Slower than baseline (>100ms)
  • ⚠️ Significant slowdown (>500ms)

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

Test Coverage Report (Windows)

Metric Value
Current Coverage 77.17%
Base Branch Coverage 77.07%
Delta 0.1% ✅

Coverage increased! Great work!

Copilot AI 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.

Pull request overview

This PR implements cross-refresh reuse of Conda environment metadata by introducing a shared, self-invalidating cache keyed on conda-meta fingerprints, and wires transient JSON-RPC refresh locator graphs to share that cache with the long-lived Conda locator (addressing #476).

Changes:

  • Added an mtime/length-based fingerprint cache for parsed Conda environment details, shared across refreshes and pruned for removed prefixes.
  • Updated refresh-locator construction to create refresh Conda locators from the shared cache owned by the long-lived server context locator.
  • Documented the updated Conda locator refresh-state behavior and added a unit test covering cross-refresh reuse and history-based invalidation.
Show a summary per file
File Description
docs/LOCATOR_STATE.md Updates the locator refresh-state contract notes for Conda to mention the shared environment-info cache.
crates/pet/src/jsonrpc.rs Creates refresh locator graphs with a Conda locator that shares the long-lived locator’s environment-info cache.
crates/pet-conda/src/lib.rs Implements the shared, fingerprint-invalidated Conda environment-info cache plus coverage validating reuse/invalidation.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment on lines +179 to +183
.read()
.expect("conda environment info cache lock poisoned")
.get(path)
.filter(|cached| &cached.fingerprint == fingerprint)
{
@karthiknadig
Karthik Nadig (karthiknadig) merged commit 31d14b0 into main Jul 25, 2026
38 checks passed
@karthiknadig
Karthik Nadig (karthiknadig) deleted the perf/issue-476 branch July 25, 2026 02:18
Karthik Nadig (karthiknadig) added a commit that referenced this pull request Aug 5, 2026
## Summary

- add a PET-wide Rust coding skill alongside locator-specific guidance
- capture path identity/cache, Unicode-safe parsing, hot-path
I/O/allocation, and cross-platform rules
- require tests to prove claimed read-count, cache-hit, and event-count
invariants
- wire the recurring checks into the Reviewer agent for every Rust
change

## Review retrospective

Recent feedback clustered around:
- normalized cache keys and caller-facing path preservation (#487, #490)
- Unicode-safe byte indexing and ASCII format handling (#493)
- proving optimization scope and read counts (#493)
- duplicate side effects and precise pattern semantics (#495)
- workflow assignment/merge postconditions, already addressed in #486

Fixes #496

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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.

Conda: find() wipes the env cache on every call, defeating cross-refresh reuse

3 participants