Skip to content

Remove zinc analysis in-memory cache - #100

Merged
sake92 merged 1 commit into
mainfrom
remove-zinc-analysis-cache
Jun 18, 2026
Merged

Remove zinc analysis in-memory cache#100
sake92 merged 1 commit into
mainfrom
remove-zinc-analysis-cache

Conversation

@sake92

@sake92 sake92 commented Jun 18, 2026

Copy link
Copy Markdown
Owner

The analysisCache saved one small disk read of inc_compile.zip per module per compilation — negligible benefit compared to actual compilation time. Deder's CachedTask already skips the entire compile task when inputs haven't changed. The setupCache (compiler classloaders/ Compilers) is preserved because it provides measurable performance value (~100-500ms savings per Scala version).

Summary by CodeRabbit

  • Refactor

    • Modified Zinc compiler's analysis caching mechanism to use disk-backed storage instead of in-memory caching for improved cache persistence and reliability.
  • Tests

    • Updated cache tests to align with revised caching behavior.

The analysisCache saved one small disk read of inc_compile.zip per
module per compilation — negligible benefit compared to actual
compilation time. Deder's CachedTask already skips the entire compile
task when inputs haven't changed. The setupCache (compiler classloaders/
Compilers) is preserved because it provides measurable performance value
(~100-500ms savings per Scala version).
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8e18447f-be6b-494d-b3e1-727a66a6d4a3

📥 Commits

Reviewing files that changed from the base of the PR and between a3bde5c and 589dd8c.

📒 Files selected for processing (3)
  • deder-common/src/ba/sake/deder/DederProjectInternals.scala
  • server/src/ba/sake/deder/zinc/ZincCompiler.scala
  • server/test/src/ba/sake/deder/zinc/ZincCompilerCacheSuite.scala

📝 Walkthrough

Walkthrough

Removes the in-memory analysisCache (Scaffeine cache of AnalysisContents) from ZincCompiler. All Zinc incremental compilation state lookups — for cross-module classpath analysis and for the current module's PreviousResult — now read exclusively from disk via ConsistentFileAnalysisStore. The corresponding doc comment and test assertions are updated to reflect the removed cache.

Changes

Remove in-memory Zinc analysis cache

Layer / File(s) Summary
Remove analysisCache and rewire disk-only reads
server/src/ba/sake/deder/zinc/ZincCompiler.scala
Deletes analysisCache field and its cacheRegistry registration; removes invalidation from close(); rewires classpathAnalysisMap construction to check disk for inc_compile.zip and read CompileAnalysis directly via ConsistentFileAnalysisStore.binary; changes previousResult selection to always call analysisStore.get(); removes post-compile write to the now-deleted cache.
Doc comment and test updates
deder-common/src/ba/sake/deder/DederProjectInternals.scala, server/test/src/ba/sake/deder/zinc/ZincCompilerCacheSuite.scala
Removes zinc-analysis:<version> from the inMemoryCachesStats documentation key list; narrows the close() test to assert only setupCache size, removing reflection-based assertions on the deleted analysisCache.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • sake92/deder#54: Modifies the same doCompile/close path in ZincCompiler.scala, making it directly adjacent to this PR's cache removal changes.

Poem

🐇 Hippity-hoppity, caches away,
The zinc analysis lived in RAM — not today!
We hop to the disk, no memory to keep,
The analysisCache is now fast asleep.
Less RAM, less fuss, the bunny agrees,
Just read from the store with the greatest of ease! 🌸

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly and accurately describes the main change: removing the zinc analysis in-memory cache from the compilation system.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove-zinc-analysis-cache

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sake92
sake92 merged commit 8e72096 into main Jun 18, 2026
2 of 4 checks passed
@sake92
sake92 deleted the remove-zinc-analysis-cache branch June 18, 2026 18: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.

1 participant