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

Make load path cache work on TruffleRuby #450

Merged
merged 2 commits into from
Jul 24, 2023

Conversation

rwstauner
Copy link
Contributor

@rwstauner rwstauner commented Jul 20, 2023

Load Path Cache can work on the latest TruffleRuby (23.1.0 is due out in September) and offers a small speedup on an example project.

It does require an unreleased bug fix (oracle/truffleruby#3131)
so I added truffleruby-head to CI to prove that it works on both the current and the next release.

I used a string comparison on RUBY_ENGINE_VERSION < "23.1.0" to do that check
which is consistent with other RUBY_VERSION >= "2.7" style comparisons used in this repo
and I think will work for the foreseeable future.

A few kernel tests are skipped due to oracle/truffleruby#3138.

A few test assertions had to be changed from "same" to "equal"
to account for TruffleRuby making copies of the strings that end up in $LOADED_FEATURES.

TruffleRuby also has a long string prefix on "builtin" features, so i added a bit of code to strip that off
so that the rest of the code is consistent. Does that make sense?

@casperisfine casperisfine marked this pull request as draft July 20, 2023 07:10
@nirvdrum nirvdrum changed the title Make load path cache work on truffleruby Make load path cache work on TruffleRuby Jul 20, 2023
@rwstauner rwstauner force-pushed the rwstauner/truffleruby-load-path-cache branch from 3cae6a7 to d443e4e Compare July 20, 2023 15:11
@rwstauner rwstauner marked this pull request as ready for review July 20, 2023 15:11
@rwstauner rwstauner force-pushed the rwstauner/truffleruby-load-path-cache branch 2 times, most recently from 66e3af0 to f0889ee Compare July 20, 2023 15:21
@rwstauner rwstauner self-assigned this Jul 20, 2023
Copy link
Contributor

@casperisfine casperisfine left a comment

Choose a reason for hiding this comment

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

A few nitpicks, but looks mostly OK to me.

lib/bootsnap/load_path_cache.rb Outdated Show resolved Hide resolved
lib/bootsnap/load_path_cache/cache.rb Outdated Show resolved Hide resolved
@rwstauner rwstauner force-pushed the rwstauner/truffleruby-load-path-cache branch 2 times, most recently from bb77e60 to d531d5a Compare July 21, 2023 20:41
@rwstauner rwstauner force-pushed the rwstauner/truffleruby-load-path-cache branch from e2eb363 to ba6e82a Compare July 24, 2023 20:31
@casperisfine casperisfine merged commit 34dba5c into main Jul 24, 2023
17 checks passed
@rwstauner rwstauner deleted the rwstauner/truffleruby-load-path-cache branch July 24, 2023 21:35
assert_same path, internal_path

if truffleruby?
assert_equal path, internal_path

Choose a reason for hiding this comment

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

Is this comparison different from CRuby due to oracle/truffleruby#3138?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, i haven't filed an issue on truffleruby for it yet, but on CRuby $LOADED_FEATURES.last is the same (frozen) string object, but on TruffleRuby this is not the case (it's a different string object, not frozen).

@shopify-shipit shopify-shipit bot temporarily deployed to rubygems July 25, 2023 06:42 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.

None yet

3 participants