Cherry-pick #80 to dev/mmtk-overrides-default #89
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bumps [ruby/setup-ruby](https://github.com/ruby/setup-ruby) from 1.188.0 to 1.190.0. - [Release notes](https://github.com/ruby/setup-ruby/releases) - [Changelog](https://github.com/ruby/setup-ruby/blob/master/release.rb) - [Commits](ruby/setup-ruby@50ba338...a6e6f86) --- updated-dependencies: - dependency-name: ruby/setup-ruby dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.25.14 to 3.25.15. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@5cf07d8...afb54ba) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
(ruby/reline#733) The allocated Fiddle::Pointer never gets freed because it doesn't have a free function defined for when it gets garbage collected. This commit changes it to use the default free function. ruby/reline@0796dcd497
Use an enum for the method arg instead of needing to add an id
that doesn't map to an actual method name.
$ ruby --dump=insns -e 'b = "x"; [v].pack("E*", buffer: b)'
before:
```
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,34)>
local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 1] b@0
0000 putchilledstring "x" ( 1)[Li]
0002 setlocal_WC_0 b@0
0004 putself
0005 opt_send_without_block <calldata!mid:v, argc:0, FCALL|VCALL|ARGS_SIMPLE>
0007 newarray 1
0009 putchilledstring "E*"
0011 getlocal_WC_0 b@0
0013 opt_send_without_block <calldata!mid:pack, argc:2, kw:[#<Symbol:0x000000000023110c>], KWARG>
0015 leave
```
after:
```
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,34)>
local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 1] b@0
0000 putchilledstring "x" ( 1)[Li]
0002 setlocal_WC_0 b@0
0004 putself
0005 opt_send_without_block <calldata!mid:v, argc:0, FCALL|VCALL|ARGS_SIMPLE>
0007 putchilledstring "E*"
0009 getlocal b@0, 0
0012 opt_newarray_send 3, 5
0015 leave
```
This is slowing down benchmarks on x86, so lets revert it for now.
Bumps [rb-sys](https://github.com/oxidize-rb/rb-sys) from 0.9.98 to 0.9.99. - [Release notes](https://github.com/oxidize-rb/rb-sys/releases) - [Commits](oxidize-rb/rb-sys@v0.9.98...v0.9.99) --- updated-dependencies: - dependency-name: rb-sys dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> rubygems/rubygems@da7b71d188
Bumps [ossf/scorecard-action](https://github.com/ossf/scorecard-action) from 2.3.3 to 2.4.0. - [Release notes](https://github.com/ossf/scorecard-action/releases) - [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md) - [Commits](ossf/scorecard-action@dc50aa9...62b2cac) --- updated-dependencies: - dependency-name: ossf/scorecard-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
This reverts commit ebc3328.
[Feature #20646]Improve Socket.tcp
This is a proposed improvement to `Socket.tcp`, which has implemented Happy Eyeballs version 2 (RFC8305) in PR9374.
1. Background
I implemented Happy Eyeballs version 2 (HEv2) for Socket.tcp in PR9374, but several issues have been identified:
- `IO.select` waits for name resolution or connection establishment in v46w, but it does not consider the case where both events occur simultaneously when it returns a value.
- In this case, Socket.tcp can only capture one event and needs to execute an unnecessary loop to capture the other one, calling `IO.select` one extra time.
- `IO.select` waits for both IPv6/IPv4 name resolution (in start), but when it returns a value, it doesn't consider the case where name resolution for both address families is complete.
- In this case, `Socket.tcp` can only obtain the addresses of one address family and needs to execute an unnecessary loop obtain the other addresses, calling `IO.select` one extra time.
- The consideration for `connect_timeout` was insufficient. After initiating one or more connections, it raises a 'user specified timeout' after the `connect_timeout` period even if there were addresses that have been resolved and have not yet tried to connect.
- It does not retry with another address in case of a connection failure.
- It executes unnecessary state transitions even when an IP address is passed as the `host` argument.
- The regex for IP addresses did not correctly specify the start and end.
2. Proposal & Outcome
To overcome the aforementioned issues, this PR introduces the following changes:
- Previously, each loop iteration represented a single state transition. This has been changed to execute all processes that meet the execution conditions within a single loop iteration.
- This prevents unnecessary repeated loops and calling `IO.select`
- Introduced logic to determine the timeout value set for `IO.select`. During the Resolution Delay and Connection Attempt Delay, the user-specified timeout is ignored. Otherwise, the timeout value is set to the larger of `resolv_timeout` and `connect_timeout`.
- This ensures that the `connect_timeout` is only detected after attempting to connect to all resolved addresses.
- Retry with another address in case of a connection failure.
- This prevents unnecessary repeated loops upon connection failure.
- Call `tcp_without_fast_fallback` when an IP address is passed as the host argument.
- This prevents unnecessary state transitions when an IP address is passed.
- Fixed regex for IP addresses.
Additionally, the code has been reduced by over 100 lines, and redundancy has been minimized, which is expected to improve readability.
3. Performance
No significant performance changes were observed in the happy case before and after the improvement.
However, improvements in state transition deficiencies are expected to enhance performance in edge cases.
```ruby
require 'socket'
require 'benchmark'
Benchmark.bmbm do |x|
x.report('fast_fallback: true') do
30.times { Socket.tcp("www.ruby-lang.org", 80) }
end
x.report('fast_fallback: false') do # Ruby3.3時点と同じ
30.times { Socket.tcp("www.ruby-lang.org", 80, fast_fallback: false) }
end
end
```
Before:
```
~/s/build ❯❯❯ ../install/bin/ruby ../ruby/test.rb
user system total real
fast_fallback: true 0.021315 0.040723 0.062038 ( 0.504866)
fast_fallback: false 0.007553 0.026248 0.033801 ( 0.533211)
```
After:
```
~/s/build ❯❯❯ ../install/bin/ruby ../ruby/test.rb
user system total real
fast_fallback: true 0.023081 0.040525 0.063606 ( 0.406219)
fast_fallback: false 0.007302 0.025515 0.032817 ( 0.418680)
```
28a1c4f seems to call an improper ensure clause. [Bug #20655] Than fixing it properly, I bet it would be much better to simply revert that commit. It reduces the unneeded complexity. Jumping into a block called by a C function like Hash#each with callcc is user's fault. It does not need serious support.
[Bug #20654]
This commit fixes Integer#floor and Float#floor when the number is
negative and ndigits is large such that 10**ndigits is a bignum.
Previously, it would return 0 in such cases. However, this would cause
unexpected behaviour such as:
puts -1.floor(-5) # => -100000
puts -1.floor(-10) # => -10000000000
puts -1.floor(-20) # => 0
This commit changes the last result so that it will return
-100000000000000000000.
[Bug #20654]
This commit fixes Integer#ceil and Float#ceil when the number is
negative and ndigits is large such that 10**ndigits is a bignum.
Previously, it would return 0 in such cases. However, this would cause
unexpected behaviour such as:
puts 1.ceil(-5) # => 100000
puts 1.ceil(-10) # => 10000000000
puts 1.ceil(-20) # => 0
This commit changes the last result so that it will return
100000000000000000000.
The tests for Integer#ceil was accidentally placed in test_truncate.
…ows a default one Previously, if you have bundler installed both as a regular gem and a default gem, the default gem would be displayed by `gem list`. rubygems/rubygems@10a6b1736e
…nt versions of the same gem Avoids warnings like ``` /path/to/ruby/3.3.4/lib/ruby/gems/3.3.0/gems/rbs-3.4.0/lib/rdoc/discover.rb:10: warning: method redefined; discarding old scan /path/to/ruby/3.3.4/lib/ruby/gems/3.3.0/gems/rbs-3.5.1/lib/rdoc/discover.rb:10: warning: previous definition of scan was here ``` ruby/rdoc@e47920d8f3
When assertions are enabled, the following code triggers an assertion
error:
GC.disable
GC.start(immediate_mark: false, immediate_sweep: false)
10_000_000.times { Object.new }
This is because the GC.start ignores that the GC is disabled and will
start incremental marking and lazy sweeping. But the assertions in
gc_marks_continue and gc_sweep_continue assert that GC is not disabled.
This commit changes it for the assertion to pass if the GC was triggered
from a method.
Bumps [rb-sys](https://github.com/oxidize-rb/rb-sys) from 0.9.100 to 0.9.101. - [Release notes](https://github.com/oxidize-rb/rb-sys/releases) - [Commits](oxidize-rb/rb-sys@v0.9.100...v0.9.101) --- updated-dependencies: - dependency-name: rb-sys dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> rubygems/rubygems@d26f5824a7
Bumps [rb-sys](https://github.com/oxidize-rb/rb-sys) from 0.9.100 to 0.9.101. - [Release notes](https://github.com/oxidize-rb/rb-sys/releases) - [Commits](oxidize-rb/rb-sys@v0.9.100...v0.9.101) --- updated-dependencies: - dependency-name: rb-sys dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> rubygems/rubygems@3addc2c3e7
Previously, proc calls such as:
```ruby
proc{|| }.(**empty_hash)
proc{|b: 1| }.(**r2k_array_with_empty_hash)
```
both allocated hashes unnecessarily, due to two separate code paths.
The first call goes through CALLER_SETUP_ARG/vm_caller_setup_keyword_hash,
and is simple to fix by not duping an empty keyword hash that will be
dropped.
The second case is more involved, in setup_parameters_complex, but is
fixed the exact same way as when the ruby2_keywords hash is not empty,
by flattening the rest array to the VM stack, ignoring the last
element (the empty keyword splat). Add a flatten_rest_array static
function to handle this case.
Update test_allocation.rb to automatically convert the method call
allocation tests to proc allocation tests, at least for the calls
that can be converted. With the code changes, all proc call
allocation tests pass, showing that proc calls and method calls
now allocate the same number of objects.
I've audited the allocation tests, and I believe that all of the low
hanging fruit has been collected. All remaining allocations are
either caller side:
* Positional splat + post argument
* Multiple positional splats
* Literal keywords + keyword splat
* Multiple keyword splats
Or callee side:
* Positional splat parameter
* Keyword splat parameter
* Keyword to positional argument conversion for methods that don't accept keywords
* ruby2_keywords method called with keywords
Reapplies abc04e8, which was reverted at
d56470a, with the addition of a bug fix and
test.
Fixes [Bug #20679]
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.26.2 to 3.26.3. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@429e197...883d858) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
This test is checking what happens if you try and define a class in a C extension where that constant is already not a class. It was doing this by overriding ::Date and then trying to require 'date. The issue with this is that if we ever add 'date' as a dependency for the test runner, this test will break because the test runner files get implicitly required in an `assert_separately` block. Better use an explicit class for this purpose which can't be accidentally required elsewhere.
The Closer and Remover finalizers are defined on different objects in Tempfile. The Closer is defined on the Tempfile object while the Remover is defined on the finalizer_obj. This means that there is no guarantee of the finalizer order. On Windows, we must close the file before removing it because we cannot remove an open file. But since the order is not guaranteed, the GC may run the Remover finalizer first, which will fail with an Errno::EACCES (Permission denied @ apply2files). This commit changes it so that both the Closer and Remover finalizers are defined on the finalizer_obj, which guarantees the order that it is ran. ruby/tempfile@eb2d8b1175
As @jeremyevans pointed out for commit eb2d8b1: > Each Tempfile instance has a separate File instance and file descriptor: > > t = Tempfile.new > t.to_i # => 6 > t.dup.to_i => 7 FinalizerManager will keep track of the open File objects for the particular file and will only unlink the file when all of the File objects have been closed. ruby/tempfile@753ab16642
Using `-rtempfile` requires the tempfile built into Ruby, not the currently developed one, so the tests aren't testing this tempfile. ruby/tempfile@ea2dec6f46
Some Ruby apps subclass Logger without running the superclass constructor, which means that `@level_override` isn't initialized properly. This can be fixed in some cases, but the gem should maintain backwards compatibility. ruby/logger@3246f38328
If `fork()` is called when having multiple threads, the child thread only inherits the forking thread. Other threads simply vanish, stopping anywhere it was executing without any cleaning up. As a result, when using MMTk, the modbufs of other threads will not be flushed, and the next nursery GC will fail to keep some root-reachable objects alive, and the program will crash later. This commit mitigates this problem by destroying mutators of dead threads after forking. Concretely, the call of `rb_mmtk_destroy_mutator` is moved into `thread_cleanup_func` which is called not only when the thread exits normally, but also after forking to clean up dead threads. That ensures the modbufs are flushed. According to the POSIX API, if `fork()` is called while there are multiple threads, the child can only call async-signal-safe functions until calling execve. There are test cases (such as `test_autoload_fork`) that forks while having multiple threads, and there may be real-world applications doing so (see https://bugs.ruby-lang.org/issues/14634). They are all unsafe according to POSIX. So this commit only mitigates the problem, and programs that fork while having multiple threads are still technically incorrect. Fixes: #83
This reverts commit 3da7e44.
If a GC is triggered by allocation, we didn't execute final jobs until
exit. This is a bug. If there are pending finalizer jobs while the
finalizer_table is empty on exit, it will trigger an assertion error.
Now we execute finalizer jobs more eagerly.
- Like the default GC, we trigger postponed job after GC in order to
run final jobs promptly.
- On exit, we ensure both finalizer_table and pending final jobs are
drained before starting to force obj_free.
Fixes: #81
Since MMTK is no longer calling it directly to scan roots, we revert it to its original form.
Yjit roots are recently added. We add a new function for MMTk, too. Also reordered the root scanning functions in the upcall table.
|
Wrong target branch. Abandoning this PR... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-pick the commit for #80 onto the dev/mmtk-overrides-default branch.