-
-
Notifications
You must be signed in to change notification settings - Fork 390
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
Spec update 20241105 #1210
Spec update 20241105 #1210
Conversation
Previously, a TypeError was not raised if there were no thread variables, because the conversion to symbol was done after that check. Convert to symbol before checking for whether thread variables are set to make the behavior consistent. Fixes [Bug #20606]
[Feature #19236] When building a large hash, pre-allocating it with enough capacity can save many re-hashes and significantly improve performance. ``` /opt/rubies/3.3.0/bin/ruby --disable=gems -rrubygems -I./benchmark/lib ./benchmark/benchmark-driver/exe/benchmark-driver \ --executables="compare-ruby::../miniruby-master -I.ext/common --disable-gem" \ --executables="built-ruby::./miniruby --disable-gem" \ --output=markdown --output-compare -v $(find ./benchmark -maxdepth 1 -name 'hash_new' -o -name '*hash_new*.yml' -o -name '*hash_new*.rb' | sort) compare-ruby: ruby 3.4.0dev (2024-03-25T11:48:11Z master f53209f023) +YJIT dev [arm64-darwin23] last_commit=[ruby/irb] Cache RDoc::RI::Driver.new (ruby/irb#911) built-ruby: ruby 3.4.0dev (2024-03-25T15:29:40Z hash-new-rb 77652b08a2) +YJIT dev [arm64-darwin23] warming up... | |compare-ruby|built-ruby| |:-------------------|-----------:|---------:| |new | 7.614M| 5.976M| | | 1.27x| -| |new_with_capa_1k | 13.931k| 15.698k| | | -| 1.13x| |new_with_capa_100k | 124.746| 148.283| | | -| 1.19x| ```
[Bug #20593] It's fairly common to use `format` to interpolate a number of values into a user provided strings. The arguments not matching are a problem when they are positional, but when they are named, it's absolutely fine and we shouldn't emit a warning.
* "Allow ambiguosity of `return` line" 65b991bc8571b7b718fc22bd33a43c4d269bf52d * "Move to test/.excludes-prism" 3b4ff810d2fefdf0194bd774bc04f6f17e2ccae7 * "Pending `EVENT_RETURN` settracefunc tests with Prism" a7f33c99c69e3cc62b7a24ce35f51f76cc5bfaa2
…r#size What a "word" is when talking about sizes is confusing because it's a highly overloaded term. Intel, Microsoft, and GDB are just a few vendors that have their own definition of what a "word" is. Specs that used the "wordsize" guard actually were mostly testing for the size of the C `long` fundamental type, so rename the guard for clarity. Also, get the size of `long` directly from RbConfig instead of guessing using Integer#size. Integer#size is not guaranteed to have anything to do with the `long` type.
There is no guarantee that Integer#size will continue to return `sizeof(long)` for small integers. Use the `l!` specifier for Array#pack instead. It is a public interface that has a direct relationship with the `long` type.
The spec is actually testing a behaviour stemming from NUM2INT(), and since `sizeof(long)>=sizeof(int)`, `min_long-1` always makes NUM2INT() raise `RangeError`.
Make Range#step to consistently use + for iteration [Feature #18368] Previously, non-numerics expected step to be integer, and iterated with begin#succ, skipping over step value steps. Since this commit, numeric and non-numeric iteration behaves the same way, by using + operator.
[Feature #20707] Converting Time into RFC3339 / ISO8601 representation is an significant hotspot for applications that serialize data in JSON, XML or other formats. By moving it into core we can optimize it much further than what `strftime` will allow. ``` compare-ruby: ruby 3.4.0dev (2024-08-29T13:11:40Z master 6b08a50a62) +YJIT [arm64-darwin23] built-ruby: ruby 3.4.0dev (2024-08-30T13:17:32Z native-xmlschema 34041ff71f) +YJIT [arm64-darwin23] warming up...... | |compare-ruby|built-ruby| |:-----------------------|-----------:|---------:| |time.xmlschema | 1.087M| 5.190M| | | -| 4.78x| |utc_time.xmlschema | 1.464M| 6.848M| | | -| 4.68x| |time.xmlschema(6) | 859.960k| 4.646M| | | -| 5.40x| |utc_time.xmlschema(6) | 1.080M| 5.917M| | | -| 5.48x| |time.xmlschema(9) | 893.909k| 4.668M| | | -| 5.22x| |utc_time.xmlschema(9) | 1.056M| 5.707M| | | -| 5.40x| ```
[Feature #20702] Works the same way than `Hash#fetch_values` for for array.
[Feature #20594] A handy method to construct a string out of multiple chunks. Contrary to `String#concat`, it doesn't do any encoding negociation, and simply append the content as bytes regardless of whether this result in a broken string or not. It's the caller responsibility to check for `String#valid_encoding?` in cases where it's needed. When passed integers, only the lower byte is considered, like in `String#setbyte`.
This is a static function only called in two places (rb_to_id and rb_to_symbol), and in both places, both symbols and strings are allowed. This makes the error message consistent with rb_check_id and rb_check_symbol. Fixes [Bug #20607]
The absence of either the integer or fractional part should be allowed.
[Feature #20205] The warning now suggests running with --debug-frozen-string-literal: ``` test.rb:3: warning: literal string will be frozen in the future (run with --debug-frozen-string-literal for more information) ``` When using --debug-frozen-string-literal, the location where the string was created is shown: ``` test.rb:3: warning: literal string will be frozen in the future test.rb:1: info: the string was created here ``` When resurrecting strings and debug mode is not enabled, the overhead is a simple FL_TEST_RAW. When mutating chilled strings and deprecation warnings are not enabled, the overhead is a simple warning category enabled check. Co-authored-by: Jean Boussier <[email protected]> Co-authored-by: Nobuyoshi Nakada <[email protected]> Co-authored-by: Jean Boussier <[email protected]>
[Bug #20803] `abc` is used a lot across the ruby spec suite, if another test runs before this spec is loaded and create this symbol dynamically (`"abc".to_sym`) the spec will fail. So it's preferable to use a symbol name that is very unlikely to be used elsewhere to avoid flakes.
* YJIT: Replace Array#each only when YJIT is enabled * Add comments about BUILTIN_ATTR_C_TRACE * Make Ruby Array#each available with --yjit as well * Fix all paths that expect a C location * Use method_basic_definition_p to detect patches * Copy a comment about C_TRACE flag to compilers * Rephrase a comment about add_yjit_hook * Give METHOD_ENTRY_BASIC flag to Array#each * Add --yjit-c-builtin option * Allow inconsistent source_location in test-spec * Refactor a check of BUILTIN_ATTR_C_TRACE * Set METHOD_ENTRY_BASIC without touching vm->running
* It was calculating remaining time once up-front, properly decrementing remaining nanos for the loop, but continuing to sleep based on the original calculation * Now it calculates the remainder on each iteration, based on the current decremented nanoseconds * Also moves the spec to a more appropriate shared ruby spec location
Fixed issues with |
It doesn't appear to be transient, though. It consistently passes on Windows and Linux and consistently fails on MacOS. And it's hard to spec
Agreed, I will move it.
I will do this for now and continue investigating the CI MacOS failures separately.
I think it would be better going forward if the person that introduced a breaking change corrects that change. @andrykonchin Thank you for correcting those issues! |
This is really a sleep spec, so it should be tested under the Kernel#sleep specs. Fails consistently on MacOS (in CI) and passes consistently on Linux and Windows so omit MacOS for now.
As I suspected, it fails in TruffleRuby's CI on Linux, both on CRuby and TruffleRuby. |
And it also has caused failures in CRuby: ruby/ruby@7c893c8 |
* See discussion on ruby/spec#1210
* See discussion on ruby/spec#1210
Have you considered the possibility that it might actually just be broken in CRuby and TruffleRuby? It has been passing in JRuby's CI consistently for several weeks. Regardless, it is behavior that was important enough for @ioquatix to file issues with both TruffleRuby and JRuby. Perhaps he can weigh in on how to verify the behavior in a more reliable way. |
I should also point out that in the pr where I am investigating the failure, CRuby isn't just off a little bit. It's off by an order of magnitude (8-35x off from expected). Something is causing a 0.1ms sleep to take way longer on Darwin. This is not just an inconvenience for testing. I think it exposes a real problem. |
Anyway, feel free to try to improve this spec or check CRuby's sleep impl. |
And I'm saying it wasn't transiently failing when I merged it over. It was consistently passing on Linux and Windows and consistently failing on Darwin by a wide margin. I went with your suggestion to leave it enabled only on the passing platforms and opened a draft PR to investigate further. I don't care if it is quarantined for now but I believe it is wrong to dismiss the failures as being due solely to unreliable sleep times. Being off by an order of magnitude is not just unreliable, it's broken. |
IIRC, the issue was If your (actual_duration > 0.01).should == true # 100 * 0.0001 => 0.01
(actual_duration < 1).should == true The first assertion seems totally reasonable to me, although technically it could be The second assertion is frustratingly problematic. On an oversubscribed system, The only reasonable way I've found to deal with this is to try and detect how over-subscribed the system is and try to compensate for it, when dealing with these kinds of tests. The other option is to simply remove assertions on upper bounds. Trying to add a multiplier of 100x is still bound to be flaky on some edge cases (bell curve etc). As a counter point, on a normal under-subscribed system (not overloaded), Bear in mind that adding a 100x factor may also hide bugs... In other words, the assertion, when run on an under-subscribed system was better for detecting issues. It's an extremely tricky issue. Maybe we need to differentiate between "hard failure" (assertion) and some kind of informational "something looks wrong". (Sorry, I'm not sure this really helps much, I've been here multiple times with |
@ioquatix thank you for the input! I agree that the excessively long sleep times on MRI seem like they're a problem to be investigated. For now, I have simply removed the I won't have time to look further into why MRI failed the 0.03 check but I think somebody should. |
@ioquatix that makes a lot of sense and matches my experiences as well. @headius Did you get some conflicts when rebasing specs? Anything non-trivial? |
Is this when running locally, e.g. under-subscribed system, or when running in CI, where we can expect it to be "problematic"? |
FWIW in oracle/truffleruby#2716 (comment) I made some measurements. |
@eregon There were a few small conflicts, I think both TruffleRuby and CRuby added the same specs. Nothing that was difficult to resolve.
@ioquatix Locally for me on MacOS and in CI on Linux and Windows it passed consistently. CI on MacOS consistently failed, with 100 sleeps of 0.1ms taking anywhere from 80ms to 350ms. Apparently TruffleRuby also had issues. JRuby passes it consistently in CI on all platforms we test (mostly Linux). |
Perhaps I'm looking at it wrong, but it seems like only TruffleRuby is having difficulties there. JRuby, now fixed, always has a quantum under in the range of 0.003 to 0.005 on my M1 Mac. It's also hard to say it's the sleep that's imprecise in your testing. If it warms up does it stabilize for TR? CRuby is always around 0.003 except when it GCs (looping over the example 10k times). |
It's the same for all Rubies, in theory they should all print
|
Using
Notice how each sleep is too long by about 50% / 0.00005s. |
My take on this is we are not using real-time operating systems, so sleep() is at least as much as passed, with pretty much no guaranteed upper bound under load or when the sleep amount is small (e.g. below 1ms). |
Nice analysis! It's great to see the original problem I had ( |
No description provided.