Skip to content

Improve pin caching - #1006

Closed
apiology wants to merge 452 commits into
castwide:v0.59from
apiology:improve_pin_caching
Closed

Improve pin caching#1006
apiology wants to merge 452 commits into
castwide:v0.59from
apiology:improve_pin_caching

Conversation

@apiology

@apiology apiology commented Jul 15, 2025

Copy link
Copy Markdown
Contributor

This PR has been broken into the following parts:

I'll keep this open and use it after these are merged to make sure the result is what I was looking for, but you can disregard this diff entirely.

Original PR description:

Way too much inside - apologies for the large monolithic change:

  1. solargraph gems caches all cacheable gems/core/stdlibs for a workspace, allowing for CI-based ahead-of-time caching
  2. Rebuild logic to deal with external bundles and add specs
    • Interested in whether this helps real-world scenarios!
    • Big caveat: all gems need to be available in the environment Solargraph runs in - maybe we could dynamically install them in the future?
  3. Performance: Define what we need from Parser gem in RBS and stop trying to process it as YARD (one-off for now, but can be made into config preferences for other projects...)
  4. Refactor things out of DocMap class:
    • Workspace::Gemspecs: Resolves gemspecs and dependencies from bundle
    • Workspace::RequirePaths: Calculates the require paths from config and .gemspec file if it exists
    • PinCache: Gem building logic
  5. 100% spec coverage of all affected lines, including more than a few new scenarios
  6. Improved resolution of requires into gems driven by specific problematic cases now in specs
  7. More linting (probably too much) to make RuboCop happy
  8. Add rspec timeouts, which were valuable figuring out some plugin-related issues in the LSP specs.

Perf note

Watching the checks, I was a little worried the plugin regression workflows had slowed down. After isolating things a little more, I think it's possible, but I'm not convinced that more data points won't average this all down to the same thing in the end. I did separate out the plugin regression workflows to tease out impact of each. Here's the data I see:

Specs:

  • solargraph-rails alone: ~5m
  • solargraph-rspec alone: ~7m
  • both plugins: ~13m
  • no plugins specs alone: ~7m

Typechecking

  • solargraph-rails alone: ~2.5m
  • solargraph-rspec alone: ~2.5m
  • both plugins: ~3m
  • no plugins: ~2.5m

I'd suggest we keep an eye on this, but I am comfortable personally moving forward with this PR in the meantime.

PR complexity note

I'm working now on splitting this into smaller PRs - as those are merged I hope to get this down small enough to be reviewed more easily.

Progress so far:

  • 2025-08-24: 62 changed files (starting place)

Next steps:

  • Merge master back in after recent PRs
  • Reproduce "Completion requests that previously took a fraction of a second now take up to 6 seconds." and write spec if feasible
  • Reproduce "Go-to-definition no longer works on gem sources." and write spec if feasible
  • Fix "Completion requests that previously took a fraction of a second now take up to 6 seconds."
  • Fix "Go-to-definition no longer works on gem sources."
  • Get CI green again
  • Merge RuboCop autocorrects #1039
  • Squash commits

@apiology
apiology force-pushed the improve_pin_caching branch 2 times, most recently from 29a7ce2 to 74ba99e Compare July 16, 2025 15:52
@apiology
apiology force-pushed the improve_pin_caching branch from ef6737e to fccacab Compare July 26, 2025 18:05
apiology added 27 commits July 26, 2025 15:09
It seems like sync_catalog will go through the motions but not
actually load pins from gems here due to passing an empty requires
array to ApiMap.

I'm sure those requires get pulled in eventually, but we go through at
least one catalog cycle without it happening.

Found while trying to test a different issue but not being able to get
completions from a gem in a spec.
@apiology
apiology changed the base branch from master to v0.59 January 28, 2026 00:04
@apiology

apiology commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #1239, a rebase of this branch onto current master.

@apiology apiology closed this Aug 2, 2026
apiology added a commit to apiology/solargraph that referenced this pull request Aug 2, 2026
CI showed the new 60s global time limit tripping on 5 specs that are
legitimately slow (gem/bundle resolution, YAML/Thor lookups) rather
than hung: ApiMap#get_method_stack's YAML/Thor cases,
ApiMap#returns core methods, Host#opens a file outside of prepared
libraries, Protocol#handles $/solargraph/documentGems, and
Gemspecs#fetch_dependencies's external-bundle cases. Tag each with the
same per-example time_limit_seconds override the original PR
(castwide#1006) settled on, so this PR doesn't regress CI
on its own before the rest of that PR's stacked branches land.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
apiology added a commit to apiology/solargraph that referenced this pull request Aug 21, 2026
Introduce Solargraph::PinCache, replacing the old class-method-based
PinCache module with an instance-based engine that owns YARD and RBS
collection caching, plus combining them into a single cached pin set
per gem. Yardoc, GemPins, and RbsMap are updated to support it:

* Yardoc splits doc-building (build_docs/build_pins) out of its old
  do-everything cache method, so PinCache can drive the build and
  caching steps separately.
* GemPins drops build_yard_pins (now owned by PinCache) and adds
  combine_method_pins_by_path for deduping method pins by path.
* RbsMap falls back to StdlibMap resolution when a gemspec isn't
  found in the RBS collection.

Also fixes a real bug in RbsMap::Conversions surfaced while extracting
this: two pairs of duplicate method definitions (parts_of_function,
build_type) where an old implementation was left in place, shadowed
and made unreachable by a newer one added elsewhere in the file. The
dead code referenced two helper methods (other_type_to_type,
method_type_to_type) that don't exist anywhere in lib/, so it would
have raised NoMethodError had it ever been called - removing it drops
this file's strong-typecheck problem count from 32 to 21 (all
pre-existing, unrelated to this change).

Extracted from castwide#1006 (Improve pin caching) as the
foundational piece of that PR: the new caching engine and its direct
collaborators, without yet wiring it into DocMap/Workspace/ApiMap or
the CLI (those follow in stacked PRs on top of this one).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
apiology added a commit to apiology/solargraph that referenced this pull request Aug 21, 2026
Replace DocMap's ad hoc gem-caching logic with delegation to PinCache
(introduced in the prior stacked PR), simplifying DocMap substantially.
Workspace gains a pin_cache accessor plus cache_gem/uncache_gem/
cache_all_for_workspace! entry points that drive PinCache for a given
workspace's gemspecs. ApiMap follows the renamed DocMap API
(cache_all! -> cache_doc_map_gems!, uncached_gemspecs.any? ->
any_uncached?) and dedupes resolved method aliases via
GemPins.combine_method_pins_by_path. Library exposes pin_cache
(delegating to workspace), uses it to check whether a gem's cache
build is already in progress, and fixes a subprocess chdir bug in its
background gem-caching thread.

Extracted from castwide#1006 (Improve pin caching) as the
second piece of that PR, stacked on top of the PinCache engine PR.
This depends on PinCache existing; the CLI updates that depend on this
wiring follow in a further stacked PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
apiology added a commit to apiology/solargraph that referenced this pull request Aug 21, 2026
Reimplement `solargraph cache`, `uncache`, and `gems` as thin wrappers
over the Workspace#cache_gem/uncache_gem/cache_all_for_workspace!
entry points added in the prior stacked PR, removing the CLI's own
duplicated build/cache logic.

Note: 2 specs in this PR ("with unbundled environments #cache
succeeds" / "#gems succeeds") will fail until
castwide#1225 merges - they exercise
Workspace::Gemspecs#find_gem in an environment with no discoverable
Gemfile, which currently raises Bundler::GemfileNotFound instead of
falling back gracefully. Verified locally that applying castwide#1225's fix
makes both pass with no other changes needed here.

Extracted from castwide#1006 (Improve pin caching) as the
final piece of that PR, stacked on top of the DocMap/Workspace wiring
PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
castwide pushed a commit that referenced this pull request Aug 26, 2026
* Improve overload resolution and macro handling in Chain::Call

Extract per-overload signature matching in Call#inferred_pins into
match_overload_type, improving how argument/block types are matched
against method overloads and how macro/directive-based pins are
reprocessed when no signature matches by type alone.

Extracted from #1006 (Improve pin caching) as a
standalone piece: this is a type-inference improvement to method call
resolution, independent of the gem pin caching machinery in the rest
of that PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Link array element-type tracking pendings to #1223

That PR restores the array/tuple literal element-type inference that
master reverted, which is what these pending specs are waiting on.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Link overload-narrowing pendings to #1246

Filed a new issue after tracing the root cause enough to size a fix:
it's not in Chain::Call's overload matching (this PR's own code) but
in how the resulting local variable's type gets resolved/cached
afterward, a different subsystem than what this PR touches.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Simplify pending reasons to a bare PR URL

Per review: when a spec is pending on a specific PR, state just
the URL, no narrative.

---------

Co-authored-by: Claude Sonnet 5 <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.

3 participants