Update: merge with master (2024-08-21) #88
Merged
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
[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
…ot include the current platform rubygems/rubygems@afb7a6d754
Directly call APIs available on Windows Vista/Server 2008 and later.
Directly call APIs available on Windows 8/Server 2012 and later.
These files were to build libffi from the bundled source, but are no longer used since we stopped bundling the libffi sources in commit e4f5296. The gemspec file is unchanged because fiddle gem itself still supports ruby 2.5.
method. (ruby/rdoc#1135) * Unify top_level creation in tests * Remove unnecessary file_name param from Parser.for It should be always the same as the top_level's absolute_name, so there's no point of taking it as a separate parameter. ruby/rdoc@97c497dfbb
For example, the following script leaks:
class MyRipper < Ripper
def initialize(src, &blk)
super(src)
@blk = blk
end
def compile_error(msg) = @blk.call(msg)
end
def call_parse = MyRipper.new("/") { |msg| return msg }.parse
10.times do
100_000.times do
call_parse
end
puts `ps -o rss= -p #{$$}`
end
Before:
93952
169040
244224
318784
394432
468224
544048
618560
693776
768384
After:
19776
19776
20352
20880
20912
21408
21328
21152
21472
20944
…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
This reverts commit 3da7e44.
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.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| # entry. | ||
| def enter(name, node, type) | ||
| line = lines[node.location.start_line - 1].chomp | ||
| pattern = "/^#{line.gsub("\\", "\\\\\\\\").gsub("/", "\\/")}$/;\"" |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
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.
rb_gc_mark_rootsfunction.