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

Update to use libv8-node 16.x #210

Merged
merged 3 commits into from
Oct 31, 2021
Merged

Update to use libv8-node 16.x #210

merged 3 commits into from
Oct 31, 2021

Conversation

lloeki
Copy link
Collaborator

@lloeki lloeki commented Jun 22, 2021

Based on V8 9.0, with node 16 being the new LTS, node 15 now being unsupported. It
notably introduces single threaded mode.

Requiring c++14 is apparently the only change needed for this major. A
separate PR will add a more helpful install time check for compiler
requirements.

@lloeki
Copy link
Collaborator Author

lloeki commented Jun 22, 2021

Do not merge yet!

libv8-node 16 hasn't been released on ruby gems (hence the test failures), as I need to finalise the CI support for cross compiling to ARM before I have public things out there (even beta).

This early branch allows select testers to proceed with testing on private gems, as well as libv8-node CI's test jobs (which are based on mini_racer) to pass.

@lloeki
Copy link
Collaborator Author

lloeki commented Jul 12, 2021

Cross-compilation of Node 16 to aarch64-linux is making good progress, so I should be publishing the gems when I get to fix a small (known) issue on musl.

@SamSaffron I have a couple of flaky test failures on x86_64-darwin CI. I think they might be due to the CI machines suffering from the occasional pressure, which makes stuff not happen in a timely manner from time to time. I'm not sure if there's a way to improve these:

Run options: --seed 6360

# Running:

...................S......S......................F........................................

Finished in 4.243624s, 21.2083 runs/s, 35.3471 assertions/s.

  1) Failure:
MiniRacerTest#test_ensure_gc [/Users/runner/work/ruby-libv8-node/ruby-libv8-node/test/mini_racer/test/mini_racer_test.rb:757]:
expecting most of the 1_000_000 long string to be freed

90 runs, 150 assertions, 1 failures, 0 errors, 2 skips
Run options: --seed 17030

# Running:

.........................F....S...............................................S...........

Finished in 4.744077s, 18.9710 runs/s, 31.6184 assertions/s.

  1) Failure:
MiniRacerTest#test_timeout_in_ruby_land [/Users/runner/work/ruby-libv8-node/ruby-libv8-node/test/mini_racer/test/mini_racer_test.rb:645]:
MiniRacer::ScriptTerminatedError expected but nothing was raised.

90 runs, 150 assertions, 1 failures, 0 errors, 2 skips

@lloeki
Copy link
Collaborator Author

lloeki commented Jul 12, 2021

Had to disable pointer compression (which is experimental) because of this commit.

@lloeki lloeki changed the title Update to use libv8-node 16.3.0 Update to use libv8-node 16.x Jul 12, 2021
@lloeki
Copy link
Collaborator Author

lloeki commented Oct 14, 2021

PR containing 16.x on the libv8-node side: rubyjs/libv8-node#24

Based on V8 9.0, his is the new LTS, node 15 now being unsupported. It
notably introduces single threaded mode.

Requiring c++14 is apparently the only change needed for this major. A
separate PR will add a more helpful install time check for compiler
requirements.
Node 16.4.0 introduced new cage flags for the experimental pointer
compression stuff. Unfortunately something fails and none of them are
set which causes the build to bail out because of the inconsistency.
@SamSaffron
Copy link
Collaborator

anything to worry about with the various compilation errors?

@lloeki
Copy link
Collaborator Author

lloeki commented Oct 19, 2021

  1. x86_64 2.6 gnu and 2.7 gnu failed because they picked up x86_64-linux-musl instead of x86_64-linux. Old rubygems issue, fixed upstream.
  2. x86_64 2.6 musl and 2.7 musl failed because they think they're x86_64-linux and points the linker to a nonexistent file. Same old rubygems issue, but manifests differently. I might be able to work around that one within libv8-node.
  3. aarch64-linux-musl libv8-node is unreleased (no cross-compiling for musl), so it picks up aarch64-linux and fails. I'll produce a build using Docker on my M1 machine.
  4. x86_64 3.0 musl failed because of this upstream ruby issue for which IIUC a fix has been merged but not released. IIRC using clang can work around that, or we can try applying the small patch on that Ruby.
  5. Not sure what happened on macOS 11.0, it seems unrelated to mini_racer:
#<Thread:0x00007fcb8d0796e8 /Users/runner/work/mini_racer/mini_racer/test/mini_racer_test.rb:587 run> terminated with exception (report_on_exception is true):
/usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:73:in `bytes': stack level too deep (SystemStackError)
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:260:in `gen_random'
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:73:in `bytes'
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:260:in `gen_random'
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:73:in `bytes'
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:260:in `gen_random'
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:73:in `bytes'
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:260:in `gen_random'
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:73:in `bytes'
	 ... 10908 levels...
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:260:in `gen_random'
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:155:in `random_bytes'
	from /usr/local/Cellar/[email protected]/2.7.4/lib/ruby/2.7.0/securerandom.rb:176:in `hex'
	from /Users/runner/work/mini_racer/mini_racer/test/mini_racer_test.rb:588:in `block (2 levels) in test_concurrent_access_over_the_same_isolate_2'

@SamSaffron
Copy link
Collaborator

@lloeki should I go ahead and merge this ? Push a new gem?

@lloeki
Copy link
Collaborator Author

lloeki commented Oct 29, 2021

I did not manage to work around the old rubygems issues on musl.

If you're OK with the issues and that if people encounter them they can pin to a previous version then go ahead, it's probably well worth it to have a recent libv8 available.

@SamSaffron
Copy link
Collaborator

I am merging this, but will wait a bit till I push gems for a bit longer

@SamSaffron SamSaffron merged commit 8bd860b into master Oct 31, 2021
@alexeevit
Copy link

@lloeki hey hi

  1. x86_64 2.6 gnu and 2.7 gnu failed because they picked up x86_64-linux-musl instead of x86_64-linux. Old rubygems issue, fixed upstream.
  2. x86_64 2.6 musl and 2.7 musl failed because they think they're x86_64-linux and points the linker to a nonexistent file. Same old rubygems issue, but manifests differently. I might be able to work around that one within libv8-node.

I'm not sure what's the difference between these points, but I ran into the issue on my x86_64 Linux VPS. It thinks it's x86_64-linux but it picked up x86-64-linux-musl. Is there a quick workaround to make it work?

@lloeki
Copy link
Collaborator Author

lloeki commented Nov 5, 2021

@alexeevit it's very subtle, see rubygems/rubygems#3174 for gory details. The true fix is not there yet but are workarounds in latest rubygems and bundler, so what's your rubygems and bundler version?

@alexeevit
Copy link

alexeevit commented Nov 5, 2021

@alexeevit it's very subtle, see rubygems/rubygems#3174 for gory details. The true fix is not there yet but are workarounds in latest rubygems and bundler, so what's your rubygems and bundler version?

$ gem -v
3.0.3.1
$ bundler -v
Bundler version 1.17.2

Then I'll try to update them first and come back with the result. Thank you!

UPD:

haha, it's such a pity the project uses rails 4.2 that depends on bundler < 2.0

@lloeki
Copy link
Collaborator Author

lloeki commented Nov 8, 2021

@alexeevit I've got a gist for you. Make sure to read the note, you probably want to use the code patch instead of the runtime monkeypatch.

https://gist.github.com/lloeki/12dcf61324f64a2fa1e8a8b2109c1f00

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.

3 participants