-
Notifications
You must be signed in to change notification settings - Fork 93
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
Use libv8 node #186
Use libv8 node #186
Conversation
After this, the only remaining issue preventing Discourse from booting on apple silicon is mini_racer/libv8. See upstream discussion at rubyjs/mini_racer#186 for an experimental solution.
After this, the only remaining issue preventing Discourse from booting on apple silicon is mini_racer/libv8. See upstream discussion at rubyjs/mini_racer#186 for an experimental solution.
I get this error when running
|
@sweetppro Might be the same as this one: |
@lloeki we now have 3 developers that had lots of luck with the the new gem. @davidtaylorhq was amongst them. Performance on Discourse is pretty much spectacular on native M1. Looking forward to getting this new pattern into a production gem! |
Sharing this finding here as well, one would really want to use Ruby >= 2.7.2 on M1, as versions before that are more or less subtly broken. |
There is no way of utilizing Bundler's (2.2) enhanced platform support to auto-switch? I mean in the Gemfile directly, I guess using a local version via |
ba5caa3
to
b5f2fb8
Compare
This causes issues on some Ruby installs, e.g AWS Linux Keeping it around for diagnostics/future configurable
Required for recent-ish C++ required by current V8
GitHub Actions results for 6b38556 here and here. It fails on aarch64-linux (except on Ruby 3.0, go figure), might be because of the build on Apple Silicon (through a VM), might be because of QEMU, or an interaction between both:
Travis seems to be fine with the build, although agonisingly slow to update rubygems. Ruby 3.0 aarch64-linux-musl fails but that might be because there's some specifics to that Docker image (I noticed the build env seems weirdly broken by default, e.g |
Do you feel we are in a good enough state to start gathering public feedback by doing a pre-release gem of mini_racer? |
Yeah, I think it'll solve more issues than it "creates" (on platforms that weren't there before). |
BTW I think we can go straight to 0.4.0 (.beta1) given the change's magnitude. |
sure! sounds good to me!
…On Wed, Apr 7, 2021 at 4:22 PM Loic Nageleisen ***@***.***> wrote:
BTW I think we can go straight to 0.4.0 (.beta1) given the change's
magnitude.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#186 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAABIXM4KHPB2K2V7K4R2V3THP24DANCNFSM4WHLB53A>
.
|
@lloeki I just pushed mini_racer 0.4.0.beta1 using node libv8! 🎊 Asking some colleagues with m1 macs to test it out! Seems to work fine on my Linux install. |
Happy do run some tests on m1. Do you want to get feedback/issues here? |
It seems to run just fine on my m1 MacBook Air. |
Thanks @SamSaffron! @tisba positive reports should be OK here in this discussion, but please create a dedicated issue if there's a problem. |
seems good! At least from a casual test: irb(main):003:0> context = MiniRacer::Context.new
=> #<MiniRacer::Context:0x000000010f8faad8 @functions={}, @timeout=nil, @max_memory=nil, @current_exception=nil, @isolate=false, @ensure_gc_after_idle=nil, @disposed=false, @ca...
irb(main):004:0> context.eval 'var adder = (a,b)=>a+b;'
=> nil
irb(main):005:0> puts context.eval 'adder(20,22)'
42
=> nil
irb(main):006:0> RUBY_PLATFORM
=> "arm64-darwin20"
irb(main):007:0> RUBY_VERSION
=> "2.7.2"
irb(main):008:0> MiniRacer::VERSION
=> "0.4.0.beta1" |
@lloeki all our devs are having great success, should we go ahead and promote both v8 and mini_racer to release? |
I have various crashes with the beta under M1 + Docker for Mac. I did not come around yet to compile details :( |
@tisba That's on aarch64-linux then? Please open a separate issue with the details. Also can you try installing the |
Yup @SamSaffron let's do this. I'll push the updated |
Using a slight modified version of #170, using |
Happy to report that the bigger test suite with a lot of mini_racer usage works fine natively on M1 using |
For me the upgrade (change from function_body = <<~EOS
function localeCompare(a, b) {
return (a || '').localeCompare((b || ''), 'en', { sensitivity: 'base' });
}
EOS
function = 'localeCompare'
inputs = [
"Rich",
"Rich"
]
expectation = 0
snapshot = MiniRacer::Snapshot.new(function_body)
context = MiniRacer::Context.new(snapshot: snapshot)
result = context.call(function, *inputs) # throws RangeError: Internal error. Icu error.
expect(result).to eq(expectation) looks like libv8-node is compiled with In the end used a good-enough polyfill function localeCompare(a, b) {
const str = (a || '').toLowerCase();
const other_str = (b || '').toLowerCase();
const lang = 'en';
if (hasWorkingICU(lang)) {
return str.localeCompare(other_str, lang, { sensitivity: 'base' });
} else {
// good-enough polyfill
const characterMaps = {
'en': '� _-,;:!?.\'"()[]{}@*/\&#%`^+<=>|~$0123456789aAbBcCdDeEfFgGhHiIjJkKlLmMnNoOpPqQrRsStTuUvVwWxXyYzZ',
};
const map = characterMaps[lang];
let charA = null;
let charB = null;
let index = 0;
const maxIndex = Math.max(str.length, other_str.length);
while (charA === charB && index < maxIndex) {
charA = str[index];
charB = other_str[index];
index++;
}
return Math.max(-1, Math.min(1, map.indexOf(charA) - map.indexOf(charB)));
}
}
function hasWorkingICU(lang) {
const hasICU = typeof Intl === 'object';
let isMissingLang = false;
if (hasICU) {
try {
'foo'.localeCompare('bar', lang);
} catch (e) {
isMissingLang = e.name === 'RangeError';
}
}
return hasICU && !isMissingLang;
} |
It's a configure/build option. I remember dropping to small-icu but can't remember the reason (either size, or a failing build). I'll try to do some builds with a bigger ICU. |
This is a PR for people to try
mini_racer
usinglibv8-node
built for Apple M1 or musl.Note: this is only tested using a
rbenv install
'd Ruby (which builds asarm64
), and untested using the system-provided Ruby (which is built asarm64e
) as I have not yet found a way to build libv8/node asarm64e
.Since I don't fully consider
libv8-node
ready for prime time yet, the original intent is not to merge this as is (if ever), but provide an easy testing ground via: