Skip to content

Commit

Permalink
Reduce the cache key to RUBY_REVISION + RUBY_PLATFORM
Browse files Browse the repository at this point in the history
Fix: #409

Various system versions were added, but as far as I could
gather it was just overly conservative.

This was reducing the usefulness of bootsnap when using
Alpine linux containers.
  • Loading branch information
byroot committed Mar 9, 2022
1 parent 5bb937a commit 163b718
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 50 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Unreleased

* Remove `uname` and other patform specific version from the cache keys. `RUBY_PLATFORM + RUBY_REVISION` should be
enough to ensure bytecode compatibility. This should improve caching for alpine based setups. See #409.

# 1.11.1

* Fix the `can't modify frozen Hash` error on load path cache mutation. See #411.
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,9 @@ Bootsnap writes a cache file containing a 64 byte header followed by the cache c
is a cache key including several fields:

* `version`, hardcoded in bootsnap. Essentially a schema version;
* `ruby_platform`, A hash of `RUBY_PLATFORM` (e.g. x86_64-linux-gnu) variable and glibc version (on Linux) or OS version (`uname -v` on BSD, macOS)
* `ruby_platform`, A hash of `RUBY_PLATFORM` (e.g. x86_64-linux-gnu) variable.
* `compile_option`, which changes with `RubyVM::InstructionSequence.compile_option` does;
* `ruby_revision`, the version of Ruby this was compiled with;
* `ruby_revision`, A hash of `RUBY_REVISION`, the exact version of Ruby;
* `size`, the size of the source file;
* `mtime`, the last-modification timestamp of the source file when it was compiled; and
* `data_size`, the number of bytes following the header, which we need to read it into a buffer.
Expand Down
48 changes: 0 additions & 48 deletions ext/bootsnap/bootsnap.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@
#include <errno.h>
#include <fcntl.h>
#include <sys/stat.h>
#ifndef _WIN32
#include <sys/utsname.h>
#endif
#ifdef __GLIBC__
#include <gnu/libc-version.h>
#endif

/* 1000 is an arbitrary limit; FNV64 plus some slashes brings the cap down to
* 981 for the cache dir */
Expand Down Expand Up @@ -205,29 +199,6 @@ bs_compile_option_crc32_set(VALUE self, VALUE crc32_v)
return Qnil;
}

/*
* We use FNV1a-64 to derive cache paths. The choice is somewhat arbitrary but
* it has several nice properties:
*
* - Tiny implementation
* - No external dependency
* - Solid performance
* - Solid randomness
* - 32 bits doesn't feel collision-resistant enough; 64 is nice.
*/
static uint64_t
fnv1a_64_iter_cstr(uint64_t h, const char *str)
{
unsigned char *s = (unsigned char *)str;

while (*s) {
h ^= (uint64_t)*s++;
h += (h << 1) + (h << 4) + (h << 5) + (h << 7) + (h << 8) + (h << 40);
}

return h;
}

static uint64_t
fnv1a_64_iter(uint64_t h, const VALUE str)
{
Expand Down Expand Up @@ -272,10 +243,6 @@ get_ruby_revision(void)
/*
* When ruby's version doesn't change, but it's recompiled on a different OS
* (or OS version), we need to invalidate the cache.
*
* We actually factor in some extra information here, to be extra confident
* that we don't try to re-use caches that will not be compatible, by factoring
* in utsname.version.
*/
static uint32_t
get_ruby_platform(void)
Expand All @@ -285,22 +252,7 @@ get_ruby_platform(void)

ruby_platform = rb_const_get(rb_cObject, rb_intern("RUBY_PLATFORM"));
hash = fnv1a_64(ruby_platform);

#ifdef _WIN32
return (uint32_t)(hash >> 32) ^ (uint32_t)GetVersion();
#elif defined(__GLIBC__)
hash = fnv1a_64_iter_cstr(hash, gnu_get_libc_version());
return (uint32_t)(hash >> 32);
#else
struct utsname utsname;

/* Not worth crashing if this fails; lose extra cache invalidation potential */
if (uname(&utsname) >= 0) {
hash = fnv1a_64_iter_cstr(hash, utsname.version);
}

return (uint32_t)(hash >> 32);
#endif
}

/*
Expand Down

0 comments on commit 163b718

Please sign in to comment.