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

Support Ruby 2.6.5 by avoiding pattern matching in call #13

Closed
ElMassimo opened this issue May 26, 2020 · 12 comments
Closed

Support Ruby 2.6.5 by avoiding pattern matching in call #13

ElMassimo opened this issue May 26, 2020 · 12 comments
Assignees

Comments

@ElMassimo
Copy link

ElMassimo commented May 26, 2020

Description 📖

Using pattern matching makes the call algorithm very elegant and robust in terms of type checking, but as a result Ruby versions lower than 2.7.0 are not supported.

In particular, this means that hanami-api can't be used with TruffleRuby, which at the moment doesn't have plans to support 2.7.0, as well as other Ruby implementations.

Performance and Adoption 🚀

To improve the adoption of hanami-api, it might be nice to avoid pattern matching at the moment despite of the uglier implementation, which would allow to use it in non-MRI rubies as well as older Ruby versions.

Ran the benchmarks in r10k, and there doesn't seem to be a significant difference in performance, which is good (the modified version was slightly faster and used slightly less memory when I tested it).

Question ❔

Is the Ruby 2.7.0 choice related to the delegation problem?

@jodosha jodosha self-assigned this May 27, 2020
@jodosha
Copy link
Member

jodosha commented May 27, 2020

@ElMassimo Thanks for your interest.

Ruby 2.7+ is a strategic choice, not a technical one.

The reason behind it's simple: starting something in 2020, with 3.0 released this year, it makes no sense for me as a maintainer to support old Ruby versions.

Given 2.7 supports pattern matching, I wanted to experiment with it, and the result is very satisfying.


More on that, we're aiming to target 2.7+ only for the other Hanami 2.x gems. Right now we're still supporting 2.3 for Hanami 1.x.

The bump to Hanami 2.x is a good opportunity to get rid of past versions. We don't want to start with Ruby versions that are about to reach EOL soonish.

2.5 EOL is planned for 2021-03-31
2.6 EOL is not planned
2.7 EOL is not planned

Do you know what will happen to TruffleRuby when MRI 3.0 will be released? That is a good question to answer. Not being able to jump from 2.6 to 2.7 compat is worrying for me.


I tried your patch with TruffleRuby 20 (truffleruby 20.1.0, like ruby 2.6.5, GraalVM CE Native [x86_64-darwin]) at fb5efa6 the specs are not passing. There is a problem with keyword arguments.

  28) Hanami::API Rack middleware with complex app scoped uses Rack middleware for nested scope w/o leading slash
      Failure/Error: @router.scope(*args, **kwargs, &blk)

      ArgumentError:
        wrong number of arguments (given 2, expected 1)
      # /Users/luca/.gem/truffleruby/2.6.5/bundler/gems/router-10eac67bdb23/lib/hanami/router.rb:370:in `scope'
      # ./lib/hanami/api.rb:249:in `scope'
      # ./spec/integration/hanami/api/middleware_spec.rb:210:in `block (5 levels) in <top (required)>'
      # ./spec/integration/hanami/api/middleware_spec.rb:203:in `initialize'
      # ./spec/integration/hanami/api/middleware_spec.rb:203:in `new'
      # ./spec/integration/hanami/api/middleware_spec.rb:203:in `api'
      # ./spec/integration/hanami/api/middleware_spec.rb:5:in `app'
      # ./spec/integration/hanami/api/middleware_spec.rb:305:in `block (5 levels) in <top (required)>'

Do you plan to use Hanami::API and TruffleRuby in production?

@ElMassimo
Copy link
Author

ElMassimo commented May 27, 2020

Thanks for the reply, it makes complete sense to focus current efforts in the upcoming versions of Ruby 👌

Regarding those failing tests, I forgot to push a keywords-related change.

I gave r10k a try, and got the following results:

runtime_with_startup
runtime
memory

In summary:

  • TruffleRuby is not yet ready for this use case.
  • The delegation problem makes it awkward to support 2.6 and 2.7 simultaneously.
  • Upgrading from MRI 2.5 or 2.6 to 2.7 should be simple for most Ruby projects.

Thanks again, and keep up the good work!

@jodosha
Copy link
Member

jodosha commented May 27, 2020

@ElMassimo Thanks for sharing the result graphs, do you still have the CSV files? Would you mind to paste the results here? Thanks in advance.

@ElMassimo
Copy link
Author

Sure thing!

Memory

app,10,100,1000,10000
truffle-hanami-api,444800,553504,576868,617432
truffle-syro,404880,404972,408836,584980
truffle-roda,405332,406044,407308,566184
mri-hanami-api,19752,19884,21820,45780
mri-syro,17304,17120,19676,51152
mri-roda,17308,17484,19060,37500

Runtime with Startup

app,10,100,1000,10000
truffle-hanami-api,3.81442,4.098243,6.251448,10.987238
truffle-syro,4.490427,8.479389,7.715385,18.149622
truffle-roda,4.491892,8.701072,9.532901,21.954956
mri-hanami-api,0.405999,0.406589,0.438853,0.765911
mri-syro,0.392245,0.462068,0.56828,0.905705
mri-roda,0.428068,0.500907,0.593409,0.829298

Runtime

app,10,100,1000,10000
truffle-hanami-api,1.1681605280027725,1.3163702840101905,2.1465650820173323,2.5220660340273753
truffle-syro,2.58289696398424,6.6690628890064545,5.898918199003674,15.345834303006995
truffle-roda,2.620491558976937,6.9781849139835685,7.755389172001742,19.280367103987373
mri-hanami-api,0.1714780000038445,0.16634800005704165,0.17094200011342764,0.19224699982441962
mri-syro,0.21350800013169646,0.29061800008639693,0.36591900000348687,0.4515189998783171
mri-roda,0.2393020000308752,0.32324899989180267,0.40955900005064905,0.501496999990195

@jodosha
Copy link
Member

jodosha commented May 27, 2020

@ElMassimo Thank you very much. How is it possible that TruffleRuby so slower if compared with MRI?

For instance, the differences for the Runtime section are impressive. That was unexpected.

EDIT: I asked on Twitter: https://twitter.com/jodosha/status/1265648619534913537?s=20

@ElMassimo
Copy link
Author

ElMassimo commented May 27, 2020

Unexpected, right? I asked in the TruffleRuby project to see if it could be related with the benchmark not running long enough, or one of the engine settings.

Testing slower frameworks like Sinatra or Rails, the tests ran for over 10 minutes, but the difference in behavior was similar. Not sure how much time is needed for it to warm up.

@jodosha
Copy link
Member

jodosha commented May 27, 2020

@ElMassimo From Twitter there is the suggestion to run the benchmark with warmup. r10k has an env var for that: R10K_WARMUP_ITERATIONS .

@enebo
Copy link

enebo commented May 27, 2020

warmup iterations will definitely help since this benchmark does n process spawns per benchmark run. Each one of those runs will start out cold again and need to warm up to show eventual performance.

@eregon
Copy link

eregon commented May 27, 2020

Thanks for the interesting issue and the performance measurements, we'll look into it in oracle/truffleruby#1939

Regarding pattern matching it would be alright to add to TruffleRuby, as it's compatible with older versions (unlike the keyword arguments changes which are still heavily discussed), as mentioned in oracle/truffleruby#2004 (comment). It's probably quite some work to implement though.

Maybe https://github.com/ruby-next/ruby-next can help for pattern matching until it's implemented in the parser & translator.

@ElMassimo
Copy link
Author

ElMassimo commented May 27, 2020

Thanks for sharing, very interesting project, funny that the transpiled pattern matching is faster than the implementation in 2.7.0.

@headius
Copy link

headius commented May 27, 2020

FWIW, JRuby will release a new version with 2.6 support, but we will NOT be moving to 2.7 soon because of the breaking changes in keyword arguments. I would strongly recommend supporting 2.6 for at least another year.

JRuby will move to 2.7 if the keyword argument situation gets settled, or to 2.8 if that happens. If there's a 3.0 this year (I personally think it would be premature) we'll look at supporting it during 2021.

@palkan
Copy link

palkan commented Jul 19, 2020

funny that the transpiled pattern matching is faster than the implementation in 2.7.0

It will be a bit better in 3.0 (btw, I used a slightly modified #call method for benchmarks).

Given 2.7 supports pattern matching, I wanted to experiment with it, and the result is very satisfying.

That's the main reason I started working on Ruby Next—to be able to use modern features w/o sacrificing compatibility with older Rubies (and alternative Rubies, too).

@jodosha If you're interested in giving Ruby Next a try, I would be glad to propose a PR. For what I see now, that's gonna be a small addition to the codebase and one more runtime dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants