Repository navigation
Vec allocation #[track_caller] should be removed #146963
Description
#126557 added #[track_caller] to lots of allocating methods on Vec.
The description of all 3 PRs are very light on details, and what they don't reveal (and their added tests can't reveal since there are none):
- This doesn't work by default, as OOM currently isn't surfaced as a panic. The OOM messages have no location whatsoever.
- Even with
-Zoom=panic, it doesn't work today, locations always point at the alloc crate.
I am a bit puzzled about 1). Was this never tested when the PR was opened or am I missing something? For 2) I didn't verify whether it worked originally, but even if it did, it then broke later. No one bothered to report its breakage, which I take as evidence that this feature is not very useful (probably due to 1)).
I don't think this feature is that useful because while it can be neat to see the location immediately, it's not that useful since backtraces do the same thing with only slightly more effort. If it was free I wouldn't mind, but it is not:
The feature had a 1-2% binary size impact, which is pretty significant (which was pointed out by @nnethercote on the PR). It meant that every inclined .push() had to get a location before calling the grow function, which is several instructions.
Test code:
Vec::<u32>::with_capacity(100000000000000);struct CookedAlloc;
unsafe impl std::alloc::GlobalAlloc for CookedAlloc {
unsafe fn alloc(&self, layout: std::alloc::Layout) -> *mut u8 {
if layout.size() > 1_0000 {
std::ptr::null_mut()
} else {
std::alloc::System.alloc(layout)
}
}
unsafe fn dealloc(&self,ptr: *mut u8,layout: std::alloc::Layout) {}
}
#[global_allocator]
static GALLOC: CookedAlloc = CookedAlloc;
fn main() {
let mut v = Vec::new();
for i in 0.. {
v.push([0; 1_000_000]);
}
}Example output (default):
memory allocation of 400000000000000 bytes failedExample output (from push, -Zoom=panic):
thread 'main' panicked at library/std/src/alloc.rs:356:9:
memory allocation of 4000000 bytes failed
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Activity
Noratrieb commentedon Sep 24, 2025
@GrigorenkoPV @nvzqz @joshtriplett since you were involved there, maybe you know something I forgot
Noratrieb commentedon Sep 24, 2025
Here's example Assembly that shows the overhead:
See
lea rsi, [rip + .Lanon.e3e90fd6d2a7c4661a1a3acf2f60bc6d.1]and the associated static allocation.nnethercote commentedon Sep 24, 2025
Vec methods like push are incredibly hot.
GrigorenkoPV commentedon Sep 24, 2025
I am fine with this being reverted. I only picked this up because I saw the PR die 3 times not based on merit, but simply due to merge conflicts.
So if the value is not really there, it makes sense to revert.
Auto merge of #147042 - Noratrieb:untrack-caller-vec, r=tgross35
Auto merge of #147042 - Noratrieb:untrack-caller-vec, r=tgross35
Auto merge of #147042 - Noratrieb:untrack-caller-vec, r=tgross35
Auto merge of rust-lang#147042 - Noratrieb:untrack-caller-vec, r=tgro…
Auto merge of #147042 - Noratrieb:untrack-caller-vec, r=tgross35