Improve __crystal_once performance

[BlobCodes]

Description

This PR significantly improves __crystal_once performance.

I originally wrote this code for a custom crystal stdlib focused on microchips without malloc.

While the previous implementation used an array to keep track of all variables currently being initialized, this one (ab)uses the given flag boolean pointer as an enum with three values to represent the "being initialized" state.

Also, the previous implementation always used a mutex when accessing any const variable using -Dpreview_mt.
This implementation instead has a fast path for the (very likely) scenario that the variable was already initialized which doesn't need a mutex.

Let's talk numbers

Benchmark code and results on my machine can be found here:
https://gitlab.com/-/snippets/4772439

[HertzDevil]

Codegen still declares an i1 for the flag in Crystal::CodeGenVisitor#declare_const_initialized_flag, there will probably be undefined behavior unless this is also changed

[BlobCodes]

Codegen still declares an i1 for the flag in Crystal::CodeGenVisitor#declare_const_initialized_flag, there will probably be undefined behavior unless this is also changed

I'm also working on the codegen part (primarily to improve performance), but I think this shouldn't have any undefined behavior since the pointer is always cast into the enum type before accessing it, and a boolean has to be at least one byte in size.

[HertzDevil]

A cast doesn't cut it because LLVM is free to assume the unused bits in the flags are poison.

[straight-shoota]

The idea sounds great, but the implementation must be sound.
I suppose we could change to a bigger integer type for real?

Also related to #14905 and #15085 which might want to use a similar mechanism.

[ysbaddaden]

@BlobCodes Very nice, thank you! 😍

@HertzDevil By poison you mean that the cast may feel safe, but that we'd be assuming some undefined behavior? LLVM doesn't make guarantees that the 7 other bits will be carried over, just that the value for an i1 can be 0 or 1?

@straight-shoota That sounds safer, that'd need a new __crystal_once2 fun (for backward compiler compatibility) and to keep the slow legacy initialization.

[straight-shoota]

need a new __crystal_once2 fun (for backward compiler compatibility) and to keep the slow legacy initialization.

We should keep the existing implementation around for compatibilty with older compilers.
But it might not be necessary to introduce a new fun. The symbol is not even used anywhere because it should always get inlined. And the compiler knows whether the second parameter is Bool or UInt8, so it can generate code approriately.

I'm wondering why this is even a fun when it's not intended to be exported. A def should be fine for this then.

[BlobCodes]

A cast doesn't cut it because LLVM is free to assume the unused bits in the flags are poison.

The main performance improvement is from the inlined fast-check, which is also possible without using the enum.

Removing the Array usage was primarily done to use __crystal_once together with --prelude=empty.
I could also create another PR with only the performance improvement.

That sounds safer, that'd need a new __crystal_once2 fun (for backward compiler compatibility) and to keep the slow legacy initialization.

Creating a new __crystal_once2 could reduce the overhead of the once mechanism even further, so that's a good idea anyways.

[straight-shoota]

Sounds like a great idea to separate the two changes into separate PRs. That's in general, and also in particular if we can merge one without any prerequisites.