search

crystal-lang / distribution-scripts

40 stars 24 forks source link

Enable PCRE2 JIT on pre-built macos binaries #228

Closed Blacksmoke16 closed 1 year ago

Blacksmoke16 commented 1 year ago

Currently this is disabled: https://github.com/crystal-lang/distribution-scripts/blob/master/omnibus/config/software/pcre2.rb#L37-L38. While it doesn't prevent anything from actually running, would be a nice perf boost.

Originally posted by @straight-shoota in https://github.com/crystal-lang/distribution-scripts/issues/224#issuecomment-1435530218

Blacksmoke16 commented 1 year ago

Looks like the root issue is ultimately https://github.com/zherczeg/sljit/blob/5ed7e00a1430ea9afc5725979b2e8053ff9d6521/sljit_src/allocator_src/sljitExecAllocatorApple.c#LL84C38-L84C44, given we're not setting that value, it's resulting in the error.

https://github.com/zherczeg/sljit/pull/127 This PR was created to make that logic that was originally added in https://github.com/zherczeg/sljit/pull/111 a bit smarter.

I was able to get it past building pcre2 at least by adding -DMAC_OS_X_VERSION_MIN_REQUIRED=110000 to CFLAGS within pcre2.rb. However based on https://crystal-lang.org/reference/1.7/syntax_and_semantics/platform_support.html#tier-1 we can't really do that given we support versions lower than that.

EDIT: This is specific to the arm64 arch part of the build, so maybe there's a way to only pass this flag for that part of it, which would be valid since it's only ever existed since OSX 11.0 anyway.

straight-shoota commented 1 year ago

Thanks for digging into this!

I suppose there must be some way to build the arm and x86 versions of the library differently. But I don't know how (maybe just build twice with a single -arch argument each and then lipo them up like the compiler binary?).

@maxfierke Any idea?

straight-shoota commented 1 year ago

I also wouldn't mind increasing the minimum supported OS version to macOS 11. The last release of macOS 10 has reached end of life last September. This would probably be the easiest solution to implement.

The compiler would effectively still support older OS releases (10.7-10.15), just the distribution binary would only work for >= 11. If you want to use the compiler with an older OS version, you have to build the library yourself.

In terms of platform support, x86_64-darwin would be in tier 1 for macOS 11+. And macOS 10.7-10.15 would be tier 2.

maxfierke commented 1 year ago

I suppose there must be some way to build the arm and x86 versions of the library differently. But I don't know how (maybe just build twice with a single -arch argument each and then lipo them up like the compiler binary?).

yeah, two separate builds w/ lipo would probably work fine if the dual architecture mode doesn't work (though it is a bit of a pain to setup)

straight-shoota commented 1 year ago

I think the general opinion in https://github.com/crystal-lang/crystal/issues/13170 is that we don't need to keep backwards compatibility with macOS < 11. If we can get that we'll certainly take it, but right now it's not a priority to spend extra effort.

@Blacksmoke16 Would you create a PR for the CFLAGS patch?