Improve pin caching - #1006
Closed
apiology wants to merge 452 commits into
Closed
Conversation
apiology
force-pushed
the
improve_pin_caching
branch
2 times, most recently
from
July 16, 2025 15:52
29a7ce2 to
74ba99e
Compare
apiology
force-pushed
the
improve_pin_caching
branch
from
July 26, 2025 18:05
ef6737e to
fccacab
Compare
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.
Contributor
Author
|
Superseded by #1239, a rebase of this branch onto current master. |
This was referenced 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
solargraph gemscaches all cacheable gems/core/stdlibs for a workspace, allowing for CI-based ahead-of-time cachingPerf 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:
Typechecking
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:
Next steps: