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

The same cache seems used for different Ruby versions #384

Closed
eregon opened this issue Jan 4, 2022 · 3 comments · Fixed by #387
Closed

The same cache seems used for different Ruby versions #384

eregon opened this issue Jan 4, 2022 · 3 comments · Fixed by #387

Comments

@eregon
Copy link
Contributor

eregon commented Jan 4, 2022

Before Shopify/yjit-bench#69 (review) I and @noahgibbs noticed that Bootsnap has an invalid cache when running the erubi_rails benchmark with e.g. 3.1-dev, 3.1.0 and 3.0.x.
It seems the cache is not separated per Ruby version.

rm -rf tmp/cache/bootsnap is a workaround, but hard to remember, and the error can be quite confusing.

Should bootsnap care about this?
It seems not so uncommon to use e.g. different Ruby releases to run a Rails app, and Rails seems to just use the default bootsnap cache path.

One issue is if CRuby dev versions are "supported" they report the same RUBY_VERSION even though many things including the ABI, the stdlib files, the bytecode format, etc might change. Maybe using RUBY_REVISION (the commit) is the only safe way then.

@casperisfine
Copy link
Contributor

👋 the ruby version is collected here:

/*
* Ruby's revision may be Integer or String. CRuby 2.7 or later uses
* Git commit ID as revision. It's String.
*/
static uint32_t
get_ruby_revision(void)
{
VALUE ruby_revision;
ruby_revision = rb_const_get(rb_cObject, rb_intern("RUBY_REVISION"));
if (RB_TYPE_P(ruby_revision, RUBY_T_FIXNUM)) {
return FIX2INT(ruby_revision);
} else {
uint64_t hash;
hash = fnv1a_64(ruby_revision);
return (uint32_t)(hash >> 32);
}
}

It's basically RUBY_REVISION.

If you think there's a better key, I'm totally open to update it.

@eregon
Copy link
Contributor Author

eregon commented Jan 10, 2022

Using RUBY_REVISION should be enough, although here a Hash of it is used.
What's the expected behavior if the RUBY_REVISION does not match when using bootsnap, e.g., in a Rails app?
I wonder if the issues we were hitting are maybe not guarded by that check/hash (e.g., the known stdlib files as in Shopify/yjit-metrics#119 (comment)).
Or maybe the hash of the revision isn't good enough, but that sounds not so likely.

@casperisfine
Copy link
Contributor

What's the expected behavior if the RUBY_REVISION does not match when using bootsnap, e.g., in a Rails app?

The cache entry contain a "version" header. If the header doesn't match, it acts as a cache miss and is regenerated.

I wonder if the issues we were hitting are maybe not guarded by that check/hash

I think you are right, that file was extracted in 3.1: ruby/ruby@12a0a89

The version guard is only for the compile cache (iseq etc). I think the error you did hit was a load path cache, which doesn't have such guard (IIRC). I'll try to dig a bit more into it later on (just got back from vacations, so I have tons of things to deal with first).

casperisfine pushed a commit that referenced this issue Jan 10, 2022
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 pushed a commit that referenced this issue Jan 10, 2022
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 pushed a commit that referenced this issue Jan 10, 2022
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.
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 a pull request may close this issue.

2 participants