Skip to content

Compare tagged/niche-filling layout and pick the best one #74069

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Jul 18, 2020

Conversation

erikdesjardins
Copy link
Contributor

Finishes up #71045, and so fixes #63866.

cc @eddyb
r? @nikomatsakis (since @eddyb wrote the first commit)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 5, 2020
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me after nits are fixed

@bors
Copy link
Collaborator

bors commented Jul 17, 2020

☔ The latest upstream changes (presumably #74395) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 17, 2020

📌 Commit 3924672 has been approved by nikomatsakis

@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 Jul 17, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 18, 2020
…sakis

Compare tagged/niche-filling layout and pick the best one

Finishes up rust-lang#71045, and so fixes rust-lang#63866.

cc @eddyb
r? @nikomatsakis (since @eddyb wrote the first commit)
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 18, 2020
…sakis

Compare tagged/niche-filling layout and pick the best one

Finishes up rust-lang#71045, and so fixes rust-lang#63866.

cc @eddyb
r? @nikomatsakis (since @eddyb wrote the first commit)
@eddyb
Copy link
Member

eddyb commented Jul 18, 2020

Thanks a lot! I've closed my original unfinished PR.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 18, 2020
…arth

Rollup of 11 pull requests

Successful merges:

 - rust-lang#72414 ( Add lazy initialization primitives to std)
 - rust-lang#74069 (Compare tagged/niche-filling layout and pick the best one)
 - rust-lang#74418 (ci: Set `shell: bash` as a default, remove duplicates)
 - rust-lang#74441 (bootstrap.py: patch RPATH on NixOS to handle the new zlib dependency.)
 - rust-lang#74444 (Add regression test for rust-lang#69414)
 - rust-lang#74448 (improper_ctypes_definitions: allow `Box`)
 - rust-lang#74449 (Test codegen of compare_exchange operations)
 - rust-lang#74450 (Fix `Safety` docs for `from_raw_parts_mut`)
 - rust-lang#74453 (Use intra-doc links in `str` and `BTreeSet`)
 - rust-lang#74457 (rustbuild: drop tool::should_install)
 - rust-lang#74464 (Use pathdiff crate)

Failed merges:

r? @ghost
@bors bors merged commit e775b4d into rust-lang:master Jul 18, 2020
@erikdesjardins erikdesjardins deleted the bad-niche branch July 18, 2020 16:15
@Mark-Simulacrum
Copy link
Member

@rust-timer make-pr-for e775b4d

rust-timer added a commit to rust-timer/rust that referenced this pull request Jul 21, 2020
Original message:
Rollup merge of rust-lang#74069 - erikdesjardins:bad-niche, r=nikomatsakis

Compare tagged/niche-filling layout and pick the best one

Finishes up rust-lang#71045, and so fixes rust-lang#63866.

cc @eddyb
r? @nikomatsakis (since @eddyb wrote the first commit)
@Mark-Simulacrum
Copy link
Member

Benchmarked in #74592, it does appear like this was a 5-10% performance regression on a large variety of crates. We should probably revert this PR. I've posted a revert here: xxx

https://perf.rust-lang.org/compare.html?start=d3df8512d2c2afc6d2e7d8b5b951dd7f2ad77b02&end=cfade73820883adf654fe34fd6b0b03a99458a51

cc @nnethercote

@Mark-Simulacrum
Copy link
Member

This turned out to not be the source of any regressions, and indeed might be a mild improvement. It will be re-landed in #74802. See also #74716 (comment) and the next few comments for discussion on this topic.

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 11, 2020
…ercote

Reland rust-lang#74069

Investigation in rust-lang#74716 has concluded that this PR is indeed not a regression (and in fact the rollup itself is not either).

This reverts the revert in rust-lang#74611.

r? @nnethercote cc @eddyb
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Niche-filling layouts are picked over tagged ones even when detrimental.
7 participants