Skip to content
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

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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