Skip to content

Consider adding Vec::try_with_capacity(_in) and friends #91913

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

Open
poliorcetics opened this issue Dec 14, 2021 · 10 comments
Open

Consider adding Vec::try_with_capacity(_in) and friends #91913

poliorcetics opened this issue Dec 14, 2021 · 10 comments
Assignees
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@poliorcetics
Copy link
Contributor

poliorcetics commented Dec 14, 2021

With try_reserve(_exact) stabilized, the following pattern is going to start cropping up (it already does in our internal code base):

let mut buf = Vec::new();
buf.try_reserve_exact(cap)?;

Would adding one or two extra methods per container; try_with_capacity(capacity: usize) -> Result<.., TryReserveError> and try_with_capacity_in(capacity: usize, alloc: A) -> Result<.., TryReserveError> be appropriate ?

Note that the two-liner above works perfectly to the best of my knowledge, it is just a question of conciseness. I also find it makes intent clearer from the start, but other may have diverging opinions on this.

Affected types would be:

  • try_with_capacity_in(capacity: usize, alloc: A) -> Result<.., TryReserveError> (see all types):
    • std::collections::VecDeque
    • std::vec::Vec
  • try_with_capacity(capacity: usize) -> Result<.., TryReserveError> (see all types):
    • std::collections::BinaryHeap
    • std::collections::VecDeque
    • std::collections::hash_map::HashMap
    • std::collections::hash_set::HashSet
    • std::ffi::OsString
    • std::io::BufReader
    • std::io::BufWriter
    • std::io::LineWriter
    • std::path::PathBuf
    • std::string::String
    • std::vec::Vec

@rustbot label T-libs C-feature-request T-libs-api

Aside: I'm not sure if this falls under A-allocators

@rustbot rustbot added C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 14, 2021
@poliorcetics
Copy link
Contributor Author

poliorcetics commented Dec 22, 2021

@rustbot claim

@poliorcetics
Copy link
Contributor Author

Related to #91789

@TennyZhuang
Copy link
Contributor

Related to #86942

@dpaoliello
Copy link
Contributor

I've added this for Vec: #95051

@kornelski
Copy link
Contributor

I'd like to revive this. Previous PR tried to add too many extra methods. Let's focus on just try_with_capacity().

There is a need for this method beyond convenience. try_reserve calls finish_grow which deliberately is not inline. However finish_grow takes old and new Layout as arguments, which adds code bloat at the call site, and does unnecesary checks for reallocation. OTOH with_capacity inlines to a very simple rustc_alloc call and an alloc error handler call, and try_with_capacity could be even leaner.

@kornelski
Copy link
Contributor

@rustbot claim

kornelski added a commit to kornelski/rust that referenced this issue Jan 30, 2024
kornelski added a commit to kornelski/rust that referenced this issue Jan 30, 2024
kornelski added a commit to kornelski/rust that referenced this issue Jan 30, 2024
kornelski added a commit to kornelski/rust that referenced this issue Jan 30, 2024
kornelski added a commit to kornelski/rust that referenced this issue Jan 31, 2024
kornelski added a commit to kornelski/rust that referenced this issue Jan 31, 2024
kornelski added a commit to kornelski/rust that referenced this issue Jan 31, 2024
kornelski added a commit to kornelski/rust that referenced this issue Jan 31, 2024
kornelski added a commit to kornelski/rust that referenced this issue Feb 9, 2024
kornelski added a commit to kornelski/rust that referenced this issue Feb 27, 2024
kornelski added a commit to kornelski/rust that referenced this issue Feb 27, 2024
kornelski added a commit to kornelski/rust that referenced this issue Feb 28, 2024
kornelski added a commit to kornelski/rust that referenced this issue Feb 28, 2024
kornelski added a commit to kornelski/rust that referenced this issue Feb 28, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 28, 2024
…nieu

Vec::try_with_capacity

Related to rust-lang#91913

Implements try_with_capacity for `Vec`, `VecDeque`, and `String`. I can follow it up with more collections if desired.

`Vec::try_with_capacity()` is functionally equivalent to the current stable:

```rust
let mut v = Vec::new();
v.try_reserve_exact(n)?
```

However, `try_reserve` calls non-inlined `finish_grow`, which requires old and new `Layout`, and is designed to reallocate memory. There is benefit to using `try_with_capacity`, besides syntax convenience, because it generates much smaller code at the call site with a direct call to the allocator. There's codegen test included.

It's also a very desirable functionality for users of `no_global_oom_handling` (Rust-for-Linux), since it makes a very commonly used function available in that environment (`with_capacity` is used much more frequently than all `(try_)reserve(_exact)`).
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Mar 1, 2024
…nieu

Vec::try_with_capacity

Related to rust-lang#91913

Implements try_with_capacity for `Vec`, `VecDeque`, and `String`. I can follow it up with more collections if desired.

`Vec::try_with_capacity()` is functionally equivalent to the current stable:

```rust
let mut v = Vec::new();
v.try_reserve_exact(n)?
```

However, `try_reserve` calls non-inlined `finish_grow`, which requires old and new `Layout`, and is designed to reallocate memory. There is benefit to using `try_with_capacity`, besides syntax convenience, because it generates much smaller code at the call site with a direct call to the allocator. There's codegen test included.

It's also a very desirable functionality for users of `no_global_oom_handling` (Rust-for-Linux), since it makes a very commonly used function available in that environment (`with_capacity` is used much more frequently than all `(try_)reserve(_exact)`).
kornelski added a commit to kornelski/rust that referenced this issue Mar 1, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 5, 2024
…nieu

Vec::try_with_capacity

Related to rust-lang#91913

Implements try_with_capacity for `Vec`, `VecDeque`, and `String`. I can follow it up with more collections if desired.

`Vec::try_with_capacity()` is functionally equivalent to the current stable:

```rust
let mut v = Vec::new();
v.try_reserve_exact(n)?
```

However, `try_reserve` calls non-inlined `finish_grow`, which requires old and new `Layout`, and is designed to reallocate memory. There is benefit to using `try_with_capacity`, besides syntax convenience, because it generates much smaller code at the call site with a direct call to the allocator. There's codegen test included.

It's also a very desirable functionality for users of `no_global_oom_handling` (Rust-for-Linux), since it makes a very commonly used function available in that environment (`with_capacity` is used much more frequently than all `(try_)reserve(_exact)`).
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Mar 7, 2024
…nieu

Vec::try_with_capacity

Related to rust-lang#91913

Implements try_with_capacity for `Vec`, `VecDeque`, and `String`. I can follow it up with more collections if desired.

`Vec::try_with_capacity()` is functionally equivalent to the current stable:

```rust
let mut v = Vec::new();
v.try_reserve_exact(n)?
```

However, `try_reserve` calls non-inlined `finish_grow`, which requires old and new `Layout`, and is designed to reallocate memory. There is benefit to using `try_with_capacity`, besides syntax convenience, because it generates much smaller code at the call site with a direct call to the allocator. There's codegen test included.

It's also a very desirable functionality for users of `no_global_oom_handling` (Rust-for-Linux), since it makes a very commonly used function available in that environment (`with_capacity` is used much more frequently than all `(try_)reserve(_exact)`).
jhpratt added a commit to jhpratt/rust that referenced this issue Mar 9, 2024
…nieu

Vec::try_with_capacity

Related to rust-lang#91913

Implements try_with_capacity for `Vec`, `VecDeque`, and `String`. I can follow it up with more collections if desired.

`Vec::try_with_capacity()` is functionally equivalent to the current stable:

```rust
let mut v = Vec::new();
v.try_reserve_exact(n)?
```

However, `try_reserve` calls non-inlined `finish_grow`, which requires old and new `Layout`, and is designed to reallocate memory. There is benefit to using `try_with_capacity`, besides syntax convenience, because it generates much smaller code at the call site with a direct call to the allocator. There's codegen test included.

It's also a very desirable functionality for users of `no_global_oom_handling` (Rust-for-Linux), since it makes a very commonly used function available in that environment (`with_capacity` is used much more frequently than all `(try_)reserve(_exact)`).
Nadrieril added a commit to Nadrieril/rust that referenced this issue Mar 9, 2024
…nieu

Vec::try_with_capacity

Related to rust-lang#91913

Implements try_with_capacity for `Vec`, `VecDeque`, and `String`. I can follow it up with more collections if desired.

`Vec::try_with_capacity()` is functionally equivalent to the current stable:

```rust
let mut v = Vec::new();
v.try_reserve_exact(n)?
```

However, `try_reserve` calls non-inlined `finish_grow`, which requires old and new `Layout`, and is designed to reallocate memory. There is benefit to using `try_with_capacity`, besides syntax convenience, because it generates much smaller code at the call site with a direct call to the allocator. There's codegen test included.

It's also a very desirable functionality for users of `no_global_oom_handling` (Rust-for-Linux), since it makes a very commonly used function available in that environment (`with_capacity` is used much more frequently than all `(try_)reserve(_exact)`).
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 9, 2024
Rollup merge of rust-lang#120504 - kornelski:try_with_capacity, r=Amanieu

Vec::try_with_capacity

Related to rust-lang#91913

Implements try_with_capacity for `Vec`, `VecDeque`, and `String`. I can follow it up with more collections if desired.

`Vec::try_with_capacity()` is functionally equivalent to the current stable:

```rust
let mut v = Vec::new();
v.try_reserve_exact(n)?
```

However, `try_reserve` calls non-inlined `finish_grow`, which requires old and new `Layout`, and is designed to reallocate memory. There is benefit to using `try_with_capacity`, besides syntax convenience, because it generates much smaller code at the call site with a direct call to the allocator. There's codegen test included.

It's also a very desirable functionality for users of `no_global_oom_handling` (Rust-for-Linux), since it makes a very commonly used function available in that environment (`with_capacity` is used much more frequently than all `(try_)reserve(_exact)`).
@c410-f3r
Copy link
Contributor

c410-f3r commented Jul 4, 2024

What is the status of this feature? IFAICT, there are no listed concerns.

@poliorcetics
Copy link
Contributor Author

From what I can tell, no one has implemented the methods nor has anyone asked for stabilization of those that were (I think).

@kornelski
Copy link
Contributor

try_with_capacity is implemented (unstable).

https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#method.try_with_capacity

The contentious issue was about potentially endless proliferation of try_ methods. This will probably remain controversial and block stabilization, until either somehow allowing try_ on everything becomes an accepted policy, or some other solution emerges that gives Rust something like method overloading or optional params to deal with all the try_ and _in combinations.

@c410-f3r
Copy link
Contributor

c410-f3r commented Jul 4, 2024

Thank you guys.

Looks like the try_ matter should be brought up to the lib team, otherwise progress won't be made.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants