rust icon indicating copy to clipboard operation
rust copied to clipboard

std: use `sync::RwLock` for internal statics

Open joboet opened this issue 1 year ago • 6 comments

Since sync::RwLock is now const-constructible, it can be used for internal statics, removing the need for sys_common::StaticRwLock. This adds some extra allocations on platforms which need to box their locks (currently SGX and some UNIX), but these will become unnecessary with the lock improvements tracked in #93740.

joboet avatar Aug 15 '22 12:08 joboet

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

rust-highfive avatar Aug 15 '22 12:08 rust-highfive

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

rustbot avatar Aug 15 '22 12:08 rustbot

I'm not a fan of making this change (or the one in https://github.com/rust-lang/rust/pull/100579). Can we instead change StaticMutex/StaticRwlock to use the locks internally on platforms where that impl isn't worse?

thomcc avatar Sep 18 '22 15:09 thomcc

It looks like we use this for the environment lock, and for the panic hook. I don't really feel like either of these are likely to be hot enough for the extra allocation to matter, so I'm not sure how much I actually care here now that I look.

r? @thomcc

thomcc avatar Sep 18 '22 16:09 thomcc

This looks fine. This does actually introduce a branch even on OSes with the good locks (due to the poison error handling). I doubt it matters but is probably worth preventing a rollup as a result.

@bors r+ rollup=never

thomcc avatar Sep 18 '22 16:09 thomcc

:pushpin: Commit 25cc0ce3c3eed6278bdeed9eddca8dc43ea55002 has been approved by thomcc

It is now in the queue for this repository.

bors avatar Sep 18 '22 16:09 bors

:hourglass: Testing commit 25cc0ce3c3eed6278bdeed9eddca8dc43ea55002 with merge 0f66914ea522ad0920154b14127447512a361cf0...

bors avatar Sep 18 '22 22:09 bors

:broken_heart: Test failed - checks-actions

bors avatar Sep 18 '22 22:09 bors

The job dist-various-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
test [run-make] src/test/run-make/emit-shared-files ... ok
test [run-make] src/test/run-make/rustdoc-scrape-examples-ordering ... ok
test [run-make] src/test/run-make/rustdoc-scrape-examples-multiple ... ok
test [run-make] src/test/run-make/thumb-none-cortex-m ... ok
Some tests failed in compiletest suite=run-make mode=run-make host=x86_64-unknown-linux-gnu target=thumbv6m-none-eabi

failures:


---- [run-make] src/test/run-make/thumb-none-qemu stdout ----
error: make failed
status: exit status: 2
command: "make"
--- stdout -------------------------------
--- stdout -------------------------------
bash script.sh
AR_thumbv7neon_unknown_linux_gnueabihf=arm-linux-gnueabihf-ar
AWS_ACCESS_KEY_ID=AKIA46X5W6CZI5DHEBFL
AWS_SECRET_ACCESS_KEY=***
BASE_COMMIT=a37499ae66ec5fc52a93d71493b78fb141c32f6b
BOOTSTRAP_CARGO=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo
BOOTSTRAP_PARENT_ID=2608
BOOTSTRAP_PYTHON=/usr/bin/python3
CARGO_HOME=/cargo
CC_aarch64_unknown_none_softfloat=aarch64-none-elf-gcc
CC_armv7a_none_eabi=arm-none-eabi-gcc
CC_armv7a_none_eabihf=arm-none-eabi-gcc
CC_mips64_unknown_linux_muslabi64=mips64-linux-gnuabi64-gcc
---
CI_JOB_NAME=dist-various-1
COMPILETEST_NEEDS_ALL_LLVM_COMPONENTS=1
CXX_thumbv7neon_unknown_linux_gnueabihf=arm-linux-gnueabihf-g++
DEPLOY=1
DOC_RUST_LANG_ORG_CHANNEL=https://doc.rust-lang.org/nightly
GITHUB_REF=refs/heads/auto
HERE=/checkout/src/test/run-make/thumb-none-qemu
HOME=/home/user
HOSTNAME=e91b0ac9d25d
HOSTNAME=e91b0ac9d25d
HOST_RPATH_DIR=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib
LC_CTYPE=C.UTF-8
LD_LIBRARY_PATH=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib
LD_LIB_PATH_ENVVAR=LD_LIBRARY_PATH
LLVM_BIN_DIR=/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/bin
LLVM_COMPONENTS=aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter bpf bpfasmparser bpfcodegen bpfdesc bpfdisassembler bpfinfo cfguard codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker dwp engine executionengine extensions filecheck frontendopenacc frontendopenmp fuzzercli fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interfacestub interpreter ipo irreader jitlink libdriver lineeditor linker lto m68k m68kasmparser m68kcodegen m68kdesc m68kdisassembler m68kinfo mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts objcopy object objectyaml option orcjit orcshared orctargetprocess passes powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo webassemblyutils windowsdriver windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info x86targetmca xray
LLVM_FILECHECK=/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/bin/FileCheck
MAIL=/var/mail/user
MAKEFLAGS=
MAKELEVEL=1
MIRRORS_BASE=https://ci-mirrors.rust-lang.org/rustc
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
***
PYTHON=/usr/bin/python3
PYTHON=/usr/bin/python3
RUN_MAKE_TARGETS=thumbv6m-none-eabi,thumbv7m-none-eabi,thumbv7em-none-eabi,thumbv7em-none-eabihf
RUSTC=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc
RUSTC_FORCE_RUSTC_VERSION=compiletest
RUSTDOC=LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/thumb-none-qemu/thumb-none-qemu:/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib" '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc' -L /checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/thumbv6m-none-eabi/lib -Clinker='arm-none-eabi-gcc'
RUST_CONFIGURE_ARGS=--musl-root-armv5te=/musl-armv5te       --musl-root-arm=/musl-arm       --musl-root-armhf=/musl-armhf       --musl-root-armv7hf=/musl-armv7hf       --musl-root-mips=/musl-mips       --musl-root-mipsel=/musl-mipsel       --musl-root-mips64=/musl-mips64       --musl-root-mips64el=/musl-mips64el       --disable-docs --set build.print-step-timings --enable-verbose-tests --set build.metrics --enable-sccache --disable-manage-submodules --enable-locked-deps --enable-cargo-native-static --set rust.codegen-units-std=1 --dist-compression-formats=xz --disable-dist-src --release-channel=nightly --enable-llvm-static-stdcpp --set rust.remap-debuginfo --debuginfo-level-std=1 --enable-missing-tools
RUST_DEMANGLER=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools-bin/rust-demangler
S=/checkout
SCCACHE_BUCKET=rust-lang-ci-sccache2
SCRIPT=python3 ../x.py --stage 2 test --host= --target thumbv6m-none-eabi,thumbv7m-none-eabi,thumbv7em-none-eabi,thumbv7em-none-eabihf src/test/run-make &&       python3 ../x.py dist --host= --target asmjs-unknown-emscripten,wasm32-unknown-emscripten,mips-unknown-linux-musl,mipsel-unknown-linux-musl,mips64-unknown-linux-muslabi64,mips64el-unknown-linux-muslabi64,arm-unknown-linux-musleabi,arm-unknown-linux-musleabihf,armv5te-unknown-linux-gnueabi,armv5te-unknown-linux-musleabi,armv7-unknown-linux-musleabihf,aarch64-unknown-none,aarch64-unknown-none-softfloat,sparc64-unknown-linux-gnu,x86_64-unknown-redox,thumbv6m-none-eabi,thumbv7m-none-eabi,thumbv7em-none-eabi,thumbv7em-none-eabihf,thumbv8m.base-none-eabi,thumbv8m.main-none-eabi,thumbv8m.main-none-eabihf,riscv32i-unknown-none-elf,riscv32imc-unknown-none-elf,riscv32imac-unknown-none-elf,riscv64imac-unknown-none-elf,riscv64gc-unknown-none-elf,armebv7r-none-eabi,armebv7r-none-eabihf,armv7r-none-eabi,armv7r-none-eabihf,thumbv7neon-unknown-linux-gnueabihf,armv7a-none-eabi
SHLVL=1
---
TOOLSTATE_REPO=https://github.com/rust-lang-nursery/rust-toolstate
TOOLSTATE_REPO_ACCESS_TOKEN=***
WORK_DIR=/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/thumb-none-qemu/thumb-none-qemu
_=/usr/bin/env
__COMPAT_LAYER=RunAsInvoker
/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/thumb-none-qemu/thumb-none-qemu /checkout/src/test/run-make/thumb-none-qemu
/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/thumb-none-qemu/thumb-none-qemu/example /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/thumb-none-qemu/thumb-none-qemu /checkout/src/test/run-make/thumb-none-qemu
--- stderr -------------------------------
--- stderr -------------------------------
+ CRATE=example
+ env
+ mkdir -p /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/thumb-none-qemu/thumb-none-qemu
+ pushd /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/thumb-none-qemu/thumb-none-qemu
+ rm -rf example
+ rm -rf example
+ cp -a /checkout/src/test/run-make/thumb-none-qemu/example .
+ pushd example
+ env RUSTC_BOOTSTRAP=1 'RUSTFLAGS=-C linker=arm-none-eabi-ld -C link-arg=-Tlink.x' /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo run --target thumbv6m-none-eabi
+ grep 'x = 42'
---
   Compiling unicode-xid v0.2.0
   Compiling syn v1.0.14
   Compiling cortex-m v0.6.2
   Compiling stable_deref_trait v1.1.1
   Compiling vcell v0.1.2
   Compiling cortex-m-rt v0.6.11
   Compiling cortex-m-semihosting v0.3.5
   Compiling r0 v0.2.2
   Compiling panic-halt v0.2.0
   Compiling volatile-register v0.2.0
   Compiling rustc_version v0.2.3
   Compiling bare-metal v0.2.5
   Compiling quote v1.0.2
   Compiling generic-array v0.12.3
   Compiling generic-array v0.12.3
   Compiling generic-array v0.13.2
   Compiling as-slice v0.1.2
   Compiling aligned v0.3.2
   Compiling cortex-m-rt-macros v0.1.7
   Compiling example v0.1.0 (/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/thumb-none-qemu/thumb-none-qemu/example)
error: custom attribute panicked
   |
10 | #[entry]
   | ^^^^^^^^
   |
   |
   = help: message: observed race condition in proc_macro2::nightly_works
warning: unused import: `core::fmt::Write`
 --> src/main.rs:3:5
  |
3 | use core::fmt::Write;
---
  |
4 | use cortex_m::asm;
  |     ^^^^^^^^^^^^^

warning: unused import: `cortex_m_semihosting as semihosting`
  |
  |
6 | use cortex_m_semihosting as semihosting;


warning: `example` (bin "example") generated 3 warnings
error: could not compile `example` due to previous error; 3 warnings emitted
make: *** [Makefile:27: all] Error 1



failures:

rust-log-analyzer avatar Sep 18 '22 23:09 rust-log-analyzer

@rustbot author

thomcc avatar Sep 19 '22 20:09 thomcc

@rustbot ready

joboet avatar Sep 19 '22 21:09 joboet

@bors r+

thomcc avatar Sep 19 '22 21:09 thomcc

:pushpin: Commit be09a4a8b2eadddadc0f3c00e40917e254ba91ab has been approved by thomcc

It is now in the queue for this repository.

bors avatar Sep 19 '22 21:09 bors

And for good measure,

@bors delegate+

thomcc avatar Sep 19 '22 21:09 thomcc

:v: @joboet can now approve this pull request

bors avatar Sep 19 '22 21:09 bors

:hourglass: Testing commit be09a4a8b2eadddadc0f3c00e40917e254ba91ab with merge 4456640f3e841eacc29fc3ecd88718c03bd0fcce...

bors avatar Sep 20 '22 16:09 bors

:broken_heart: Test failed - checks-actions

bors avatar Sep 20 '22 16:09 bors

That looks like a spurious failure... @bors r=thomcc

joboet avatar Sep 20 '22 17:09 joboet

:bulb: This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
  • There's another pull request that is currently being tested, blocking this pull request: #101989

bors avatar Sep 20 '22 17:09 bors

:pushpin: Commit be09a4a8b2eadddadc0f3c00e40917e254ba91ab has been approved by thomcc

It is now in the queue for this repository.

bors avatar Sep 20 '22 17:09 bors

:hourglass: Testing commit be09a4a8b2eadddadc0f3c00e40917e254ba91ab with merge 7743aa836ee16d04831a34ee1ff109bf9d411277...

bors avatar Sep 20 '22 22:09 bors

:sunny: Test successful - checks-actions Approved by: thomcc Pushing 7743aa836ee16d04831a34ee1ff109bf9d411277 to master...

bors avatar Sep 21 '22 00:09 bors

Finished benchmarking commit (7743aa836ee16d04831a34ee1ff109bf9d411277): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

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.

mean[^1] range count[^2]
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-5.3% [-5.3%, -5.3%] 1
Improvements ✅
(secondary)
-2.8% [-3.5%, -1.5%] 3
All ❌✅ (primary) -5.3% [-5.3%, -5.3%] 1

Cycles

Results

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.

mean[^1] range count[^2]
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.3% [-2.5%, -2.0%] 2
All ❌✅ (primary) - - 0

[^1]: the arithmetic mean of the percent change [^2]: number of relevant changes

rust-timer avatar Sep 21 '22 02:09 rust-timer

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

rust-log-analyzer avatar Sep 21 '22 06:09 rust-log-analyzer