Store deduped paths in $LOADED_FEATURES

[rwstauner]

This is probably naive and incomplete, but I managed to get it to work and now I can share it to ask how it should be done.

I noticed that bootsnap has some tests where it assumes the items in $LOADED_FEATURES are deduplicated strings:
https://github.com/Shopify/bootsnap/blob/2ea29bed2c027d8eb6a008d78db4a46ce68d1fa4/test/load_path_cache/cache_test.rb#L173-L181

It seems to be the case in CRuby so I added a spec for it and wanted to see what it would take in TruffleRuby.
I did find some existing comments about it so it seems like something we wanted to pursue here.

I don't even know if I'm using the right terminology (deduped strings)
and this is the first time I encountered the post-boot stuff
so I'd be glad for any feedback or direction in how to accomplish this correctly.

$LOAD_PATH also contains frozen strings in ruby, but i didn't dig further to figure out how that might be accomplished. If you have any pointers I'd be glad to integrate that.

Thanks!

[eregon]

So this is mostly to pass that bootsnap test?

I'm not sure what's the advantage of interning entries of $LOADED_FEATURES, interning/deduplicating is extra hash lookups, so should be slower overall.

I suppose one advantage is if those paths/strings are added in a Hash it would avoid an extra .dup.freeze, but that's not relevant since these are already frozen (so the extra .dup.freeze is already avoided) and we don't currently have a Hash with such keys, at least not in Truffle::FeatureLoader (it's keyed by basename without extension instead).

[rwstauner]

Yeah, i saw it in the bootsnap tests and realized it was an incompatibility, so I looked into it.
When I saw the existing "TODO" comments I figured there was some interest in it, so I pursued it.
Not sure how much value it really adds 🤷

I can't get the native image to build though, so it's more complicated than what I've toyed with so far. 😕

[eregon]

Action item here is figuring out if Bootsnap needs this or if we should just skip that test in Bootsnap when running on TruffleRuby. cc @byroot @nirvdrum

So far it seems overhead to me without benefit, so then I'd suggest skip the test in Bootsnap.
Unless the load path caching relies on this?

[rwstauner]

The tests are currently skipped (or altered) in bootsnap on TruffleRuby
https://github.com/Shopify/bootsnap/blob/main/test/load_path_cache/cache_test.rb#L174-L181

As far as I have seen it seems to provide some benefit as it is (without this PR),
but I haven't dug into too much of the internals of bootsnap.

I don't have a compelling argument for keeping this open.

[byroot]

So I added this test in Shopify/bootsnap@3a5b6af

This behavior is not strictly required, this was an optimization. The idea was to pre-massge the path strings so that Ruby won't have to dup them.

This test not passing on TruffleRuby just means a minor perf loss, no big deal.

[byroot]

Also: Shopify/bootsnap#348