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

Add CI/benchmarks for Ruby 3.3 #335

Merged
merged 2 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
matrix:
# Due to https://github.com/actions/runner/issues/849, we have to use
# quotes for '3.0' -- without quotes, CI sees '3' and runs the latest.
ruby: [2.5, 2.6, 2.7, '3.0', 3.1, 3.2, jruby, truffleruby-head]
ruby: [2.5, 2.6, 2.7, '3.0', 3.1, 3.2, 3.3, jruby, truffleruby-head]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand All @@ -24,6 +24,10 @@ jobs:
- name: Set bundler environment variables
run: |
echo "BUNDLE_WITH=checks:docs" >> $GITHUB_ENV
if: matrix.ruby == 3.3
- name: Set bundler environment variables
run: |
echo "BUNDLE_WITH=dokaz" >> $GITHUB_ENV
if: matrix.ruby == 3.2

# Use 'bundler-cache: true' instead of actions/cache as advised:
Expand All @@ -41,18 +45,19 @@ jobs:
fail_ci_if_error: true # optional (default = false)
token: ${{ secrets.CODECOV_TOKEN }}
verbose: true # optional (default = false)
if: matrix.ruby == 3.2
if: matrix.ruby == 3.3

- run: bundle exec rubocop
if: matrix.ruby == 3.2
if: matrix.ruby == 3.3

- run: |
bundle exec yard doctest
bundle exec dokaz
if: matrix.ruby == 3.2
- run: bundle exec yard doctest
if: matrix.ruby == 3.3

- run: bundle exec dokaz
if: matrix.ruby == 3.2 # Does not yet work on Ruby 3.3+: https://github.com/zverok/dokaz/issues/3

- name: Run benchmarks on Ruby 2.7 or 3.2
- name: Run benchmarks on Ruby 2.7 or 3.3
run: |
BUNDLE_GEMFILE=benchmarks/Gemfile bundle install --jobs 4 --retry 3
BUNDLE_GEMFILE=benchmarks/Gemfile bundle exec ruby benchmarks/benchmarks.rb
if: matrix.ruby == '2.7' || matrix.ruby == '3.2'
if: matrix.ruby == '2.7' || matrix.ruby == '3.3'
2 changes: 1 addition & 1 deletion .ruby-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.2.2
3.3.2
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ _No breaking changes!_

**Project enhancements:**

- Updated official test coverage to support Ruby 3.3 [[#335](https://github.com/panorama-ed/memo_wise/pull/335)]

## [v1.9.0](https://github.com/panorama-ed/memo_wise/compare/v1.8.0...v1.9.0)

**Gem enhancements:**
Expand Down Expand Up @@ -70,7 +72,7 @@ _No breaking changes!_

**Project enhancements:**

- Update official test coverage to support Ruby 3.1 [[#263](https://github.com/panorama-ed/memo_wise/pull/263)]
- Updated official test coverage to support Ruby 3.1 [[#263](https://github.com/panorama-ed/memo_wise/pull/263)]

## [v1.5.0](https://github.com/panorama-ed/memo_wise/compare/v1.4.0...v1.5.0) - 2021-12-17

Expand Down
6 changes: 5 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ end

# Excluded from CI except on latest MRI Ruby, to reduce compatibility burden
group :docs, optional: true do
gem "dokaz", "~> 0.0.5"
gem "redcarpet", "~> 3.6"
gem "webrick", "~> 1.8"
gem "yard", "~> 0.9"
gem "yard-doctest", "~> 0.1"
end

# Excluded from CI except on the latest Ruby version that supports dokaz
group :dokaz, optional: true do
gem "dokaz", "~> 0.0.5"
end

# Optional, only used locally to release to rubygems.org
group :release, optional: true do
gem "rake"
Expand Down
47 changes: 27 additions & 20 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
GIT
remote: https://github.com/panorama-ed/panolint-ruby.git
revision: a93988ea554177cf0ec9ef636c442f9d3af49a10
revision: f3fde1ecdaec4090c200346b4aa8c35c0ad13135
branch: main
specs:
panolint-ruby (0)
rubocop (= 1.51.0)
rubocop-performance (= 1.18.0)
rubocop-rspec (= 2.22.0)
rubocop (= 1.64.1)
rubocop-performance (= 1.20.2)
rubocop-rspec (= 2.29.2)

PATH
remote: .
Expand All @@ -24,11 +24,14 @@ GEM
ansi
rouge (~> 4)
slop (~> 3)
json (2.6.3)
json (2.7.2)
language_server-protocol (3.17.0.3)
minitest (5.18.0)
parallel (1.23.0)
parser (3.2.2.1)
parallel (1.24.0)
parser (3.3.2.0)
ast (~> 2.4.1)
racc
racc (1.8.0)
rainbow (3.1.1)
rake (13.1.0)
redcarpet (3.6.0)
Expand All @@ -49,29 +52,33 @@ GEM
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.13.0)
rspec-support (3.13.1)
rubocop (1.51.0)
rubocop (1.64.1)
json (~> 2.3)
language_server-protocol (>= 3.17.0)
parallel (~> 1.10)
parser (>= 3.2.0.0)
parser (>= 3.3.0.2)
rainbow (>= 2.2.2, < 4.0)
regexp_parser (>= 1.8, < 3.0)
rexml (>= 3.2.5, < 4.0)
rubocop-ast (>= 1.28.0, < 2.0)
rubocop-ast (>= 1.31.1, < 2.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 2.4.0, < 3.0)
rubocop-ast (1.28.1)
parser (>= 3.2.1.0)
rubocop-capybara (2.18.0)
rubocop-ast (1.31.3)
parser (>= 3.3.1.0)
rubocop-capybara (2.20.0)
rubocop (~> 1.41)
rubocop-factory_bot (2.23.1)
rubocop (~> 1.33)
rubocop-performance (1.18.0)
rubocop (>= 1.7.0, < 2.0)
rubocop-ast (>= 0.4.0)
rubocop-rspec (2.22.0)
rubocop (~> 1.33)
rubocop-factory_bot (2.25.1)
rubocop (~> 1.41)
rubocop-performance (1.20.2)
rubocop (>= 1.48.1, < 2.0)
rubocop-ast (>= 1.30.0, < 2.0)
rubocop-rspec (2.29.2)
rubocop (~> 1.40)
rubocop-capybara (~> 2.17)
rubocop-factory_bot (~> 2.22)
rubocop-rspec_rails (~> 2.28)
rubocop-rspec_rails (2.28.3)
rubocop (~> 1.40)
ruby-progressbar (1.13.0)
simplecov (0.22.0)
docile (~> 1.1)
Expand Down
22 changes: 11 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,19 +114,19 @@ For more usage details, see our detailed [documentation](#documentation).

Benchmarks are run in GitHub Actions, and the tables below are updated with every code change. **Values >1.00x represent how much _slower_ each gem’s memoized value retrieval is than the latest commit of `MemoWise`**, according to [`benchmark-ips`](https://github.com/evanphx/benchmark-ips) (2.11.0).

Results using Ruby 3.2.2:
Results using Ruby 3.3.2:

|Method arguments|`Dry::Core`\* (1.0.1)|`Memery` (1.5.0)|
|--|--|--|
|`()` (none)|0.66x|3.54x|
|`(a)`|1.48x|8.49x|
|`(a, b)`|1.18x|6.52x|
|`(a:)`|1.53x|13.57x|
|`(a:, b:)`|1.27x|10.56x|
|`(a, b:)`|1.26x|10.44x|
|`(a, *args)`|0.78x|1.60x|
|`(a:, **kwargs)`|0.77x|2.12x|
|`(a, *args, b:, **kwargs)`|0.69x|1.40x|
|`()` (none)|0.60x|3.17x|
Copy link
Member Author

@JacobEvelyn JacobEvelyn Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty surprised by the performance degradation here—worse results basically across the board. I don't have time to investigate though. @ms-ati do you?

|`(a)`|1.01x|7.94x|
|`(a, b)`|0.85x|6.38x|
|`(a:)`|1.00x|11.78x|
|`(a:, b:)`|0.88x|9.67x|
|`(a, b:)`|0.83x|9.44x|
|`(a, *args)`|0.67x|1.45x|
|`(a:, **kwargs)`|0.68x|1.88x|
|`(a, *args, b:, **kwargs)`|0.64x|1.29x|

\* `Dry::Core`
[may cause incorrect behavior caused by hash collisions](https://github.com/dry-rb/dry-core/issues/63).
Expand Down Expand Up @@ -180,7 +180,7 @@ versions:

We maintain API documentation using [YARD](https://yardoc.org/), which is
published automatically at
[RubyDoc.info](https://rubydoc.info/gems/memo_wise).
[RubyDoc.info](https://rubydoc.info/gems/memo_wise).

To generate documentation locally or run documentation tests,
first install the `docs` dependencies (e.g. `yard`) as follows:
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/benchmarks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def benchmark_name

# Use metaprogramming to ensure that each class is created in exactly the
# the same way.
BENCHMARK_GEMS.each do |benchmark_gem|
BENCHMARK_GEMS.each do |benchmark_gem| # rubocop:disable Metrics/BlockLength
eval <<~HEREDOC, binding, __FILE__, __LINE__ + 1 # rubocop:disable Security/Eval
class #{benchmark_gem.klass}Example
#{benchmark_gem.activation_code}
Expand Down
13 changes: 6 additions & 7 deletions lib/memo_wise.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

# Disable RuboCop here because Ruby < 3.2 does not load `set` by default.
require "set" # rubocop:disable Lint/RedundantRequireStatement
require "set" # Ruby < 3.2 does not load `set` by default.

require "memo_wise/internal_api"
require "memo_wise/version"
Expand Down Expand Up @@ -31,12 +30,12 @@ module MemoWise
# [calling the original](https://medium.com/@jeremy_96642/ruby-method-auditing-using-module-prepend-4f4e69aacd95)
# constructor.
#
# - **Q:** Why is [Module#prepend](https://ruby-doc.org/3.2.2/Module.html#method-i-prepend)
# - **Q:** Why is [Module#prepend](https://ruby-doc.org/3.3.1/Module.html#method-i-prepend)
# important here
# ([more info](https://medium.com/@leo_hetsch/ruby-modules-include-vs-prepend-vs-extend-f09837a5b073))?
# - **A:** To set up *mutable state* inside the instance, even if the original
# constructor will then call
# [Object#freeze](https://ruby-doc.org/3.2.2/Object.html#method-i-freeze).
# [Object#freeze](https://ruby-doc.org/3.3.1/Object.html#method-i-freeze).
#
# This approach supports memoization on frozen (immutable) objects -- for
# example, classes created by the
Expand Down Expand Up @@ -101,7 +100,7 @@ def inherited(subclass)
# @param target [Class]
# The `Class` into to prepend the MemoWise methods e.g. `memo_wise`
#
# @see https://ruby-doc.org/3.2.2/Module.html#method-i-prepend
# @see https://ruby-doc.org/3.3.1/Module.html#method-i-prepend
#
# @example
# class Example
Expand All @@ -116,7 +115,7 @@ class << target
#
# This is necessary in addition to the `#initialize` method definition
# above because
# [`Class#allocate`](https://ruby-doc.org/3.2.2/Class.html#method-i-allocate)
# [`Class#allocate`](https://ruby-doc.org/3.3.1/Class.html#method-i-allocate)
# bypasses `#initialize`, and when it's used (e.g.,
# [in ActiveRecord](https://github.com/rails/rails/blob/a395c3a6af1e079740e7a28994d77c8baadd2a9d/activerecord/lib/active_record/persistence.rb#L411))
# we still need to be able to access MemoWise's instance variable. Despite
Expand Down Expand Up @@ -268,7 +267,7 @@ def #{method_name}(#{MemoWise::InternalAPI.args_str(method)})
)
end

# Override [Module#instance_method](https://ruby-doc.org/3.2.2/Module.html#method-i-instance_method)
# Override [Module#instance_method](https://ruby-doc.org/3.3.1/Module.html#method-i-instance_method)
# to proxy the original `UnboundMethod#parameters` results. We want the
# parameters to reflect the original method in order to support callers
# who want to use Ruby reflection to process the method parameters,
Expand Down
2 changes: 1 addition & 1 deletion spec/dokaz_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
require "memo_wise"

# Enable the example RSpec `after` call in README.md to execute successfully
def after(scope, &)
def after(scope, &block)
# Do nothing
end
22 changes: 11 additions & 11 deletions spec/memo_wise_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,11 @@
expect(target.proc_method_counter).to eq(1)
end

it "will not memoize methods with implicit block arguments" do
it "does not memoize methods with implicit block arguments" do
expect { target.implicit_block_method }.to raise_error(LocalJumpError)
end

it "will not memoize methods with explicit block arguments" do
it "does not memoize methods with explicit block arguments" do
expect { target.explicit_block_method { nil } }.to raise_error(LocalJumpError)
end
end
Expand All @@ -153,7 +153,7 @@

# Now confirm our non-memoized method is not memoized.
expect(Array.new(4) { non_memoized.no_args }).to all eq("#{non_memoized_name}_no_args")
expect(non_memoized.send("#{non_memoized_name}_no_args_counter")).to eq(4)
expect(non_memoized.send(:"#{non_memoized_name}_no_args_counter")).to eq(4)
end
end

Expand All @@ -167,7 +167,7 @@
expect(Array.new(4) { non_memoized.with_one_positional_arg(1) }).
to all eq("#{non_memoized_name}_with_one_positional_arg: a=1")

expect(non_memoized.send("#{non_memoized_name}_one_positional_arg_counter")).to eq(4)
expect(non_memoized.send(:"#{non_memoized_name}_one_positional_arg_counter")).to eq(4)
end
end

Expand All @@ -181,7 +181,7 @@
expect(Array.new(4) { non_memoized.with_positional_args(1, 2) }).
to all eq("#{non_memoized_name}_with_positional_args: a=1, b=2")

expect(non_memoized.send("#{non_memoized_name}_positional_args_counter")).to eq(4)
expect(non_memoized.send(:"#{non_memoized_name}_positional_args_counter")).to eq(4)
end
end
end
Expand Down Expand Up @@ -884,8 +884,8 @@ def no_args_counter
end

it "doesn't define #inherited" do
expect(klass).to be_respond_to(:inherited)
expect(klass.new).to_not be_respond_to(:inherited)
expect(klass).to respond_to(:inherited)
expect(klass.new).to_not respond_to(:inherited)
end
end

Expand Down Expand Up @@ -921,8 +921,8 @@ def no_args_counter
end

it "doesn't define #inherited" do
expect(klass).to be_respond_to(:inherited)
expect(klass.new).to_not be_respond_to(:inherited)
expect(klass).to respond_to(:inherited)
expect(klass.new).to_not respond_to(:inherited)
end
end

Expand Down Expand Up @@ -960,8 +960,8 @@ def no_args_counter
end

it "doesn't define #inherited" do
expect(klass).to be_respond_to(:inherited)
expect(klass.new).to_not be_respond_to(:inherited)
expect(klass).to respond_to(:inherited)
expect(klass.new).to_not respond_to(:inherited)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/support/define_methods_for_testing_memo_wise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def self.define_methods_for_testing_memo_wise(target:, via:)

# :nocov:
raise ArgumentError, "Not a Class or Module: #{target.inspect}" unless target.is_a?(Module)
raise ArgumentError, "Unknown value for 'via': #{via}" unless %i[instance self_dot].include?(via) # rubocop:disable Layout/EmptyLineAfterGuardClause
raise ArgumentError, "Unknown value for 'via': #{via}" unless %i[instance self_dot].include?(via)
# :nocov:

self_prefix = via == :self_dot ? "self." : ""
Expand Down