Skip to content

Improve creation of 3 IndexVecs #55755

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

Jump to bottom
Merged
merged 1 commit into from
Nov 9, 2018
Merged

Improve creation of 3 IndexVecs #55755

merged 1 commit into from
Nov 9, 2018

Conversation

ljedrz
Copy link
Contributor

@ljedrz ljedrz commented Nov 7, 2018

  • preallocate when the minimal size is known
  • use from_elem_n instead of new+resize

@rust-highfive
Copy link
Contributor

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 7, 2018
varkor
varkor reviewed Nov 8, 2018
View reviewed changes
@ljedrz ljedrz force-pushed the a_few_indexvec_tweaks branch from 5811012 to e3390d8 Compare November 8, 2018 10:57
@varkor
Copy link
Member

varkor commented Nov 8, 2018

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 8, 2018

📌 Commit e3390d8 has been approved by varkor

@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 Nov 8, 2018
pnkfelix
pnkfelix reviewed Nov 8, 2018
View reviewed changes
src/librustc_mir/shim.rs
@@ -196,7 +196,7 @@ fn build_drop_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let source_info = SourceInfo { span, scope: OUTERMOST_SOURCE_SCOPE };

let return_block = BasicBlock::new(1);
let mut blocks = IndexVec::new();
let mut blocks = IndexVec::with_capacity(2);
Copy link
Member

Choose a reason for hiding this comment

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

hmm do we want/need a SmallIndexVec that is analogous to SmallVec ...?

Copy link
Member

Choose a reason for hiding this comment

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

or ... should we try something bonkers like changing IndexVec to use a SmallVec internally...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could submit an experimental PR to check the perf implications.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is harder than I thought; changing the underlying structure of IndexVec to a SmallVec makes Mir a recursive type of infinite size due to the promoted component.

kennytm added a commit to kennytm/rust that referenced this pull request Nov 8, 2018
@kennytm
Rollup merge of rust-lang#55755 - ljedrz:a_few_indexvec_tweaks, r=varkor
e182bff 
Improve creation of 3 IndexVecs

- preallocate when the minimal size is known
- use `from_elem_n` instead of `new`+`resize`
@kennytm kennytm mentioned this pull request Nov 8, 2018
Closed
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Nov 9, 2018
@Mark-Simulacrum
Rollup merge of rust-lang#55755 - ljedrz:a_few_indexvec_tweaks, r=varkor
660340e 
Improve creation of 3 IndexVecs

- preallocate when the minimal size is known
- use `from_elem_n` instead of `new`+`resize`
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Nov 9, 2018
Merged
bors added a commit that referenced this pull request Nov 9, 2018
@bors
Auto merge of #55803 - Mark-Simulacrum:rollup, r=Mark-Simulacrum
36a50c2 
Rollup of 17 pull requests

Successful merges:

 - #55576 (Clarify error message for -C opt-level)
 - #55633 (Support memcpy/memmove with differing src/dst alignment)
 - #55638 (Fix ICE in msg_span_from_free_region on ReEmpty)
 - #55659 (rustc: Delete grouping logic from the musl target)
 - #55719 (Sidestep link error from rustfix'ed code by using a *defined* static.)
 - #55736 (Elide anon lifetimes in conflicting impl note)
 - #55739 (Consume optimization fuel from the MIR inliner)
 - #55742 (Avoid panic when matching function call)
 - #55753 (borrow_set: remove a helper function and a clone it uses)
 - #55755 (Improve creation of 3 IndexVecs)
 - #55758 ([regression - rust2018]: unused_mut lint false positives on nightly)
 - #55760 (Remove intermediate font specs)
 - #55761 (mir: remove a hacky recursive helper function)
 - #55774 (wasm32-unknown-emscripten expects the rust_eh_personality symbol)
 - #55777 (Use `Lit` rather than `P<Lit>` in `ast::ExprKind`.)
 - #55783 (Deprecate mpsc channel selection)
 - #55788 (rustc: Request ansi colors if stderr isn't a tty)

Failed merges:

r? @ghost
@bors bors merged commit e3390d8 into rust-lang:master Nov 9, 2018
@ljedrz ljedrz deleted the a_few_indexvec_tweaks branch November 9, 2018 09:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@pnkfelix pnkfelix pnkfelix left review comments

@varkor varkor varkor left review comments

Assignees

@varkor varkor

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ljedrz @rust-highfive @varkor @bors @pnkfelix