Skip to content

Conversation

@apiology
Copy link
Contributor

@apiology apiology commented Jul 15, 2025

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
Copy link
Contributor Author

apiology commented Sep 9, 2025

  • Completion requests that previously took a fraction of a second now take up to 6 seconds.
  • Go-to-definition no longer works on gem sources.

@castwide: I fixed a big perf regression in the last commit (and in the extracted version in #1064). I verified I could go-to-definition on a gem source - would you be so kind as to retest to make sure there was nothing else causing those problems?

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.

2 participants