Skip to content

Rollup of 3 pull requests #115281

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

Merged
merged 6 commits into from
Aug 27, 2023
Merged

Rollup of 3 pull requests #115281

merged 6 commits into from
Aug 27, 2023

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

nbdd0121 and others added 6 commits August 18, 2023 17:44
This flag is intended for evaluation of trait upcasting
space cost for embedded use cases.
Add an (perma-)unstable option to disable vtable vptr

This flag is intended for evaluation of trait upcasting space cost for embedded use cases.

Compared to the approach in rust-lang#112355, this option provides a way to evaluate end-to-end cost of trait upcasting. Rationale: rust-lang#112355 (comment)

## How this flag should be used (after merge)

Build your project with and without `-Zno-trait-vptr` flag. If you are using cargo, set `RUSTFLAGS="-Zno-trait-vptr"` in the environment variable. You probably also want to use `-Zbuild-std` or the binary built may be broken. Save both binaries somewhere.

### Evaluate the space cost

The option has a direct and indirect impact on vtable space usage. Directly, it gets rid of the trait vptr entry needed to store a pointer to a vtable of a supertrait. (IMO) this is a small saving usually. The larger saving usually comes with the indirect saving by eliminating the vtable of the supertrait (and its parent).

Both impacts only affects vtables (notably the number of functions monomorphized should , however where vtable reside can depend on your relocation model. If the relocation model is static, then vtable is rodata (usually stored in Flash/ROM together with text in embedded scenario). If the binary is relocatable, however, the vtable will live in `.data` (more specifically, `.data.rel.ro`), and this will need to reside in RAM (which may be a more scarce resource in some cases), together with dynamic relocation info living in readonly segment.

For evaluation, you should run `size` on both binaries, with and without the flag. `size` would output three columns, `text`, `data`, `bss` and the sum `dec` (and it's hex version). As explained above, both `text` and `data` may change. `bss` shouldn't usually change. It'll be useful to see:
* Percentage change in text + data (indicating required flash/ROM size)
* Percentage change in data + bss (indicating required RAM size)
…=Mark-Simulacrum

replace outdated github username 'ozkanonur'
…mats, r=notriddle

Unify CSS color formats a bit more

When `rgb` format doesn't have an `rgba` equivalent, I turned it into hex format. I also "decapitalized" hex formats.

r? `@notriddle`
@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Aug 27, 2023
@GuillaumeGomez
Copy link
Member Author

@bors r+ p=3 rollup=never

@bors
Copy link
Collaborator

bors commented Aug 27, 2023

📌 Commit 84891c2 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 27, 2023
@bors
Copy link
Collaborator

bors commented Aug 27, 2023

⌛ Testing commit 84891c2 with merge 8550f15...

@bors
Copy link
Collaborator

bors commented Aug 27, 2023

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 8550f15 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 27, 2023
@bors bors merged commit 8550f15 into rust-lang:master Aug 27, 2023
@rustbot rustbot added this to the 1.74.0 milestone Aug 27, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#114974 Add an (perma-)unstable option to disable vtable vptr 4577a8f305fa09414e8d3d5a634d44d1f3880da6 (link)
#115261 replace outdated github username 'ozkanonur' 8cc44b5adf7b557b5049787b5e28ceb1628e3ea6 (link)
#115266 Unify CSS color formats a bit more 7a2b60486663d56b25a26a8cb1fc0712047dea00 (link)

previous master: 668bf8c593

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@GuillaumeGomez GuillaumeGomez deleted the rollup-yr05a54 branch August 27, 2023 21:55
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8550f15): 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 range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.5% [3.5%, 3.5%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.4% [-1.4%, -1.4%] 1
All ❌✅ (primary) - - 0

Cycles

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

Binary size

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

Bootstrap: 631.179s -> 630.803s (-0.06%)
Artifact size: 315.90 MiB -> 316.08 MiB (0.06%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants