fix handling of negative priorities #2087
Closed
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
This match dynamically modifies the timer thread period to account for negative priorities. Until now, the VM would not switch away from threads with negative priorities at the expected period, since the timer thread would be always waiting for a static TIME_QUANTUM_USEC of 100ms. To test this, I wrote a script that simply samples time and priority in two threads and displays the expected and actual periods. Before this patch, the result would be: 1 ran for 93.61 ms, expected 50.00 ms (prio=-1) 0 ran for 99.33 ms, expected 100.00 ms (prio=0) 1 ran for 100.08 ms, expected 50.00 ms (prio=-1) 0 ran for 100.10 ms, expected 100.00 ms (prio=0) 1 ran for 100.06 ms, expected 50.00 ms (prio=-1) 0 ran for 100.15 ms, expected 100.00 ms (prio=0) 1 ran for 99.98 ms, expected 50.00 ms (prio=-1) 0 ran for 100.09 ms, expected 100.00 ms (prio=0) 1 ran for 143.08 ms, expected 50.00 ms (prio=-1) 0 ran for 57.19 ms, expected 100.00 ms (prio=0) 1 ran for 99.95 ms, expected 25.00 ms (prio=-2) 0 ran for 136.81 ms, expected 50.00 ms (prio=-1) 1 ran for 63.32 ms, expected 25.00 ms (prio=-2) 0 ran for 100.10 ms, expected 50.00 ms (prio=-1) 1 ran for 100.04 ms, expected 25.00 ms (prio=-2) 0 ran for 100.09 ms, expected 50.00 ms (prio=-1) 1 ran for 100.49 ms, expected 25.00 ms (prio=-2) 0 ran for 130.23 ms, expected 50.00 ms (prio=-1) 1 ran for 69.48 ms, expected 25.00 ms (prio=-2) 0 ran for 100.09 ms, expected 50.00 ms (prio=-1) 1 ran for 176.32 ms, expected 25.00 ms (prio=-2) 0 ran for 23.80 ms, expected 50.00 ms (prio=-1) 1 ran for 100.04 ms, expected 12.00 ms (prio=-3) 0 ran for 203.00 ms, expected 200.00 ms (prio=1) 1 ran for 193.71 ms, expected 12.00 ms (prio=-3) 0 ran for 103.58 ms, expected 200.00 ms (prio=1) 1 ran for 34.62 ms, expected 12.00 ms (prio=-3) 0 ran for 106.44 ms, expected 200.00 ms (prio=1) 1 ran for 99.02 ms, expected 12.00 ms (prio=-3) 0 ran for 200.10 ms, expected 200.00 ms (prio=1) After the patch: 1 ran for 50.13 ms, expected 50.00 ms (prio=-1) 0 ran for 100.26 ms, expected 100.00 ms (prio=0) 1 ran for 50.01 ms, expected 50.00 ms (prio=-1) 0 ran for 100.19 ms, expected 100.00 ms (prio=0) 1 ran for 59.21 ms, expected 50.00 ms (prio=-1) 0 ran for 90.90 ms, expected 100.00 ms (prio=0) 1 ran for 65.29 ms, expected 50.00 ms (prio=-1) 0 ran for 84.13 ms, expected 100.00 ms (prio=0) 1 ran for 25.91 ms, expected 50.00 ms (prio=-1) 0 ran for 100.21 ms, expected 100.00 ms (prio=0) 1 ran for 50.04 ms, expected 50.00 ms (prio=-1) 0 ran for 100.20 ms, expected 100.00 ms (prio=0) 1 ran for 50.22 ms, expected 50.00 ms (prio=-1) 0 ran for 100.14 ms, expected 50.00 ms (prio=-1) 1 ran for 25.08 ms, expected 25.00 ms (prio=-2) 0 ran for 50.19 ms, expected 50.00 ms (prio=-1) 1 ran for 25.08 ms, expected 25.00 ms (prio=-2) 0 ran for 50.20 ms, expected 50.00 ms (prio=-1) 1 ran for 25.10 ms, expected 25.00 ms (prio=-2) 0 ran for 107.87 ms, expected 50.00 ms (prio=-1) 1 ran for 17.47 ms, expected 25.00 ms (prio=-2) 0 ran for 50.10 ms, expected 50.00 ms (prio=-1) 1 ran for 25.03 ms, expected 25.00 ms (prio=-2) 0 ran for 50.18 ms, expected 50.00 ms (prio=-1) 1 ran for 25.15 ms, expected 25.00 ms (prio=-2) 0 ran for 28.81 ms, expected 50.00 ms (prio=-1) 1 ran for 17.38 ms, expected 25.00 ms (prio=-2) 0 ran for 49.98 ms, expected 50.00 ms (prio=-1) 1 ran for 24.96 ms, expected 25.00 ms (prio=-2) 0 ran for 50.18 ms, expected 50.00 ms (prio=-1) 1 ran for 25.10 ms, expected 25.00 ms (prio=-2) 0 ran for 50.20 ms, expected 50.00 ms (prio=-1) 1 ran for 32.38 ms, expected 25.00 ms (prio=-2) 0 ran for 42.85 ms, expected 50.00 ms (prio=-1) 1 ran for 25.09 ms, expected 25.00 ms (prio=-2) 0 ran for 100.35 ms, expected 50.00 ms (prio=-1) 1 ran for 25.19 ms, expected 25.00 ms (prio=-2) 0 ran for 207.48 ms, expected 200.00 ms (prio=1) 1 ran for 11.33 ms, expected 12.00 ms (prio=-3) 0 ran for 201.17 ms, expected 200.00 ms (prio=1) 1 ran for 12.58 ms, expected 12.00 ms (prio=-3) 0 ran for 326.26 ms, expected 200.00 ms (prio=1) 1 ran for 1.43 ms, expected 12.00 ms (prio=-3) 0 ran for 226.93 ms, expected 200.00 ms (prio=1) 1 ran for 12.60 ms, expected 12.00 ms (prio=-3)
eileencodes
added a commit
to eileencodes/ruby
that referenced
this pull request
Jan 16, 2024
In ruby#2087 it was noted that there was a bug in the number of arguments in `SplatNode` and `KeywordHashNode`. I looked into this with Aaron before the linked PR was merged and we found a bunch of cases that weren't working quite right. This PR aims to fix some of those cases, but there may be more. A splat argument followed by a positional argument will concat the array until the end or unless the argument is a kwarg or splat kwarg. For example ``` foo(a, *b, c, *d, e) ``` Will have an `argc` of 2, because `b`, `c`, `d`, and `e` will be concatenated together. ``` foo(a, *b, c, *d, **e) ``` Will have an `argc` of 3, because `b`, `c`, and `d` will be concatenated together and `e` is a separate argument.
kddnewton
pushed a commit
that referenced
this pull request
Jan 17, 2024
In #2087 it was noted that there was a bug in the number of arguments in `SplatNode` and `KeywordHashNode`. I looked into this with Aaron before the linked PR was merged and we found a bunch of cases that weren't working quite right. This PR aims to fix some of those cases, but there may be more. A splat argument followed by a positional argument will concat the array until the end or unless the argument is a kwarg or splat kwarg. For example ``` foo(a, *b, c, *d, e) ``` Will have an `argc` of 2, because `b`, `c`, `d`, and `e` will be concatenated together. ``` foo(a, *b, c, *d, **e) ``` Will have an `argc` of 3, because `b`, `c`, and `d` will be concatenated together and `e` is a separate argument.
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.
Patch for https://bugs.ruby-lang.org/issues/15438