Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore load path caches generated by a different ruby version #387

Merged
merged 1 commit into from Jan 10, 2022

Conversation

casperisfine
Copy link
Contributor

Fix: #384

Until now it was assume that if the $LOAD_PATH was identical, then the content of the paths would be too.

However from one minor ruby version to another, the layout of the stdlib can change. So if the newer version is installed in the same place than the previous one, it might cause the load path cache to be invalid.

So we store RUBY_DESCRIPTION in the cache and make sure it matches before reusing the cache.

cc @eregon @noahgibbs if you mind giving this a quick look.

@eregon
Copy link
Contributor

eregon commented Jan 10, 2022

Should it warn if the load path cache can't be used due to different RUBY_DESCRIPTION?
Maybe only in $VERBOSE mode or so.

Having the cache per RUBY_REVISION might be nicer and more in line with the compile cache behavior, not sure how much work that is though 😅

In any case, RUBY_DESCRIPTION seems problematic. It does typically include a short git hash of RUBY_REVISION, so it's precise enough but it's too precise, e.g., using --yjit adds +YJIT in RUBY_DESCRIPTION.
So RUBY_REVISION seems better.

@casperisfine
Copy link
Contributor Author

Should it warn if the load path cache can't be used due to different RUBY_DESCRIPTION?

I don't think that's particularly useful. Bootsnap regularly expect caches to be invalidated.

e.g., using --yjit adds +YJIT in

That's a good point. I'll use the same key than the compile cache.

@casperisfine
Copy link
Contributor Author

Hum, I just remembered that what I liked too in RUBY_DESCRIPTION is that it contains the platform. E.g. if you try to use the same cache on your host and some kind of linux container and that somehow the ruby install path is the same (very unlikely, but might as well handle it). But I can use RUBY_REVISION + RUBY_PLATFORM

@casperisfine
Copy link
Contributor Author

Done. @eregon if you want to give it another look.

Copy link
Contributor

@eregon eregon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for the fix.

@eregon
Copy link
Contributor

eregon commented Jan 10, 2022

Couple review questions:

  • I don't see where the version is saved in the cache, just where it's read
  • Is it OK to not use an existing cache from a previous Bootsnap version? Will it be overridden at the end of the run or so?

@casperisfine
Copy link
Contributor Author

  • I don't see where the version is saved in the cache, just where it's read

If the cache is missing or the version doesn't match, we initialize it with the version key. This cache is never really mutated, it's kind of generated all at once.

Is it OK to not use an existing cache from a previous Bootsnap version? Will it be overridden at the end of the run or so?

Yes it's fine. we could add the bootsnap version in there, but so far the cache format hasn't changed. In theory you never need to purge the bootsnap cache, bootsnap takes care of doing it itself.

Fix: #384

Until now it was assume that if the `$LOAD_PATH` was identical,
then the content of the paths would be too.

However from one minor ruby version to another, the layout of the
stdlib can change. So if the newer version is installed in the same
place than the previous one, it might cause the load path cache to
be invalid.

So we store `RUBY_DESCRIPTION` in the cache and make sure it matches
before reusing the cache.
@casperisfine casperisfine merged commit 728845d into master Jan 10, 2022
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems January 10, 2022 12:35 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems January 12, 2022 21:44 Inactive
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.

The same cache seems used for different Ruby versions
3 participants