Rollup of 7 pull requests #131573
Merged
Rollup of 7 pull requests #131573
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 commit is a followup to rust-lang#124032. It replaces the tests that test the various sort functions in the standard library with a test-suite developed as part of https://github.com/Voultapher/sort-research-rs. The current tests suffer a couple of problems: - They don't cover important real world patterns that the implementations take advantage of and execute special code for. - The input lengths tested miss out on code paths. For example, important safety property tests never reach the quicksort part of the implementation. - The miri side is often limited to `len <= 20` which means it very thoroughly tests the insertion sort, which accounts for 19 out of 1.5k LoC. - They are split into to core and alloc, causing code duplication and uneven coverage. - The randomness is not repeatable, as it relies on `std::hash::RandomState::new().build_hasher()`. Most of these issues existed before rust-lang#124032, but they are intensified by it. One thing that is new and requires additional testing, is that the new sort implementations specialize based on type properties. For example `Freeze` and non `Freeze` execute different code paths. Effectively there are three dimensions that matter: - Input type - Input length - Input pattern The ported test-suite tests various properties along all three dimensions, greatly improving test coverage. It side-steps the miri issue by preferring sampled approaches. For example the test that checks if after a panic the set of elements is still the original one, doesn't do so for every single possible panic opportunity but rather it picks one at random, and performs this test across a range of input length, which varies the panic point across them. This allows regular execution to easily test inputs of length 10k, and miri execution up to 100 which covers significantly more code. The randomness used is tied to a fixed - but random per process execution - seed. This allows for fully repeatable tests and fuzzer like exploration across multiple runs. Structure wise, the tests are previously found in the core integration tests for `sort_unstable` and alloc unit tests for `sort`. The new test-suite was developed to be a purely black-box approach, which makes integration testing the better place, because it can't accidentally rely on internal access. Because unwinding support is required the tests can't be in core, even if the implementation is, so they are now part of the alloc integration tests. Are there architectures that can only build and test core and not alloc? If so, do such platforms require sort testing? For what it's worth the current implementation state passes miri `--target mips64-unknown-linux-gnuabi64` which is big endian. The test-suite also contains tests for properties that were and are given by the current and previous implementations, and likely relied upon by users but weren't tested. For example `self_cmp` tests that the two parameters `a` and `b` passed into the comparison function are never references to the same object, which if the user is sorting for example a `&mut [Mutex<i32>]` could lead to a deadlock. Instead of using the hashed caller location as rand seed, it uses seconds since unix epoch / 10, which given timestamps in the CI should be reasonably easy to reproduce, but also allows fuzzer like space exploration.
llvm/llvm-project@fa789df renamed getDeclaration to getOrInsertDeclaration. @rustbot label: +llvm-main
…ess-ids, r=aDotInTheVoid rustdoc-json: change item ID's repr from a string to an int Following [this discussion on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/Optimizing.20the.20.60Id.60.20type.20in.20.60rustdoc-types.60), I've changed the repr of `rustdoc_json_types::Id` from a String to a u32, by adding a `clean::ItemId` interner to `JsonRenderer` r? ``@aDotInTheVoid``
…=thomcc Port sort-research-rs test suite to Rust stdlib tests This PR is a followup to rust-lang#124032. It replaces the tests that test the various sort functions in the standard library with a test-suite developed as part of https://github.com/Voultapher/sort-research-rs. The current tests suffer a couple of problems: - They don't cover important real world patterns that the implementations take advantage of and execute special code for. - The input lengths tested miss out on code paths. For example, important safety property tests never reach the quicksort part of the implementation. - The miri side is often limited to `len <= 20` which means it very thoroughly tests the insertion sort, which accounts for 19 out of 1.5k LoC. - They are split into to core and alloc, causing code duplication and uneven coverage. - ~~The randomness is tied to a caller location, wasting the space exploration capabilities of randomized testing.~~ The randomness is not repeatable, as it relies on `std::hash::RandomState::new().build_hasher()`. Most of these issues existed before rust-lang#124032, but they are intensified by it. One thing that is new and requires additional testing, is that the new sort implementations specialize based on type properties. For example `Freeze` and non `Freeze` execute different code paths. Effectively there are three dimensions that matter: - Input type - Input length - Input pattern The ported test-suite tests various properties along all three dimensions, greatly improving test coverage. It side-steps the miri issue by preferring sampled approaches. For example the test that checks if after a panic the set of elements is still the original one, doesn't do so for every single possible panic opportunity but rather it picks one at random, and performs this test across a range of input length, which varies the panic point across them. This allows regular execution to easily test inputs of length 10k, and miri execution up to 100 which covers significantly more code. The randomness used is tied to a fixed - but random per process execution - seed. This allows for fully repeatable tests and fuzzer like exploration across multiple runs. Structure wise, the tests are previously found in the core integration tests for `sort_unstable` and alloc unit tests for `sort`. The new test-suite was developed to be a purely black-box approach, which makes integration testing the better place, because it can't accidentally rely on internal access. Because unwinding support is required the tests can't be in core, even if the implementation is, so they are now part of the alloc integration tests. Are there architectures that can only build and test core and not alloc? If so, do such platforms require sort testing? For what it's worth the current implementation state passes miri `--target mips64-unknown-linux-gnuabi64` which is big endian. The test-suite also contains tests for properties that were and are given by the current and previous implementations, and likely relied upon by users but weren't tested. For example `self_cmp` tests that the two parameters `a` and `b` passed into the comparison function are never references to the same object, which if the user is sorting for example a `&mut [Mutex<i32>]` could lead to a deadlock. Instead of using the hashed caller location as rand seed, it uses seconds since unix epoch / 10, which given timestamps in the CI should be reasonably easy to reproduce, but also allows fuzzer like space exploration. --- Test run-time changes: Setup: ``` Linux 6.10 rustc 1.83.0-nightly (f79a912 2024-09-18) AMD Ryzen 9 5900X 12-Core Processor (Zen 3 micro-architecture) CPU boost enabled. ``` master: e9df22f Before core integration tests: ``` $ LD_LIBRARY_PATH=build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/ hyperfine build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/coretests-219cbd0308a49e2f Time (mean ± σ): 869.6 ms ± 21.1 ms [User: 1327.6 ms, System: 95.1 ms] Range (min … max): 845.4 ms … 917.0 ms 10 runs # MIRIFLAGS="-Zmiri-disable-isolation" to get real time $ MIRIFLAGS="-Zmiri-disable-isolation" ./x.py miri library/core finished in 738.44s ``` After core integration tests: ``` $ LD_LIBRARY_PATH=build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/ hyperfine build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/coretests-219cbd0308a49e2f Time (mean ± σ): 865.1 ms ± 14.7 ms [User: 1283.5 ms, System: 88.4 ms] Range (min … max): 836.2 ms … 885.7 ms 10 runs $ MIRIFLAGS="-Zmiri-disable-isolation" ./x.py miri library/core finished in 752.35s ``` Before alloc unit tests: ``` LD_LIBRARY_PATH=build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/ hyperfine build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/alloc-19c15e6e8565aa54 Time (mean ± σ): 295.0 ms ± 9.9 ms [User: 719.6 ms, System: 35.3 ms] Range (min … max): 284.9 ms … 319.3 ms 10 runs $ MIRIFLAGS="-Zmiri-disable-isolation" ./x.py miri library/alloc finished in 322.75s ``` After alloc unit tests: ``` LD_LIBRARY_PATH=build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/ hyperfine build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/alloc-19c15e6e8565aa54 Time (mean ± σ): 97.4 ms ± 4.1 ms [User: 297.7 ms, System: 28.6 ms] Range (min … max): 92.3 ms … 109.2 ms 27 runs $ MIRIFLAGS="-Zmiri-disable-isolation" ./x.py miri library/alloc finished in 309.18s ``` Before alloc integration tests: ``` $ LD_LIBRARY_PATH=build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/ hyperfine build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/alloctests-439e7300c61a8046 Time (mean ± σ): 103.2 ms ± 1.7 ms [User: 135.7 ms, System: 39.4 ms] Range (min … max): 99.7 ms … 107.3 ms 28 runs $ MIRIFLAGS="-Zmiri-disable-isolation" ./x.py miri library/alloc finished in 231.35s ``` After alloc integration tests: ``` $ LD_LIBRARY_PATH=build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/ hyperfine build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/alloctests-439e7300c61a8046 Time (mean ± σ): 379.8 ms ± 4.7 ms [User: 4620.5 ms, System: 1157.2 ms] Range (min … max): 373.6 ms … 386.9 ms 10 runs $ MIRIFLAGS="-Zmiri-disable-isolation" ./x.py miri library/alloc finished in 449.24s ``` In my opinion the results don't change iterative library development or CI execution in meaningful ways. For example currently the library doc-tests take ~66s and incremental compilation takes 10+ seconds. However I only have limited knowledge of the various local development workflows that exist, and might be missing one that is significantly impacted by this change.
…exhaustive, r=joboet Stabilize `debug_more_non_exhaustive` Fixes: rust-lang#127942
stabilize const_result Waiting for FCP to complete in rust-lang#82814 Fixes rust-lang#82814
… r=RalfJung Stabilise `const_char_encode_utf8`. Closes: rust-lang#130512 This PR stabilises the `const_char_encode_utf8` feature gate (i.e. support for `char::encode_utf8` in const scenarios). Note that the linked tracking issue is currently awaiting FCP.
…henkov coverage: Remove code related to LLVM 17 In-tree LLVM is 19, and the minimum external LLVM was increased to 18 in rust-lang#130487.
…tion, r=cuviper RustWrapper: adapt for rename of Intrinsic::getDeclaration llvm/llvm-project@fa789df renamed getDeclaration to getOrInsertDeclaration. `@rustbot` label: +llvm-main
rustbot
added
A-rustdoc-json
|
@bors r+ rollup=never p=7 |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
📌 Perf builds for each rolled up PR:
previous master: 1bc403daad In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
This was referenced Oct 12, 2024
|
Finished benchmarking commit (fb20e4d): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (secondary 2.4%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 782.608s -> 781.192s (-0.18%) |
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.
Successful merges:
debug_more_non_exhaustive#131109 (Stabilizedebug_more_non_exhaustive)const_char_encode_utf8. #131463 (Stabiliseconst_char_encode_utf8.)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup