Err automatic hint::cold_path?

language design
1

Looking at this convert function, I was surprised that it doesn’t have a hint::cold_path. IMHO both consuming and producing an error, doubly make the second branch a candidate. OTOH, if it didn’t have the const-hack, a ? would offer no place to put the hint. That makes me wonder, whether just like for panic!() that happens automatically.

pub const fn from_utf8(v: &[u8]) -> Result<&str, Utf8Error> {
    // FIXME(const-hack): This should use `?` again, once it's `const`
    match run_utf8_validation(v) {
        Ok(_) => {
            // SAFETY: validation succeeded.
            Ok(unsafe { from_utf8_unchecked(v) })
        }
        Err(err) => Err(err),
    }
}

That raises the question: what about None. That might be more common than Err. But is it common enough to make an automatically inserted hint::cold_path counterproductive?

2

It's not obvious to me that this is the right thing for the standard library to do.

Notably, things like u32::from_str it's not obvious that the Err being cold is necessarily the right choice. And adding anything in ? is at risk of a pretty bad compilation-time hit. (IIRC serde isn't using ? because its generality and From made it materially slower to compile, even if it optimized away.)

So overall, I generally think that leaving this kind of thing to PGO is the better choice.

I definitely don't think doing it for Option would be good, because things like try { x?.foo() } being used on stuff that happens to be None is pretty common when it's used as "this property/hook/etc really is optional". Maybe it'd work in the Err arm for Result::branch? But that'd also make try { x? } no longer a no-op, which has its own consequences. You could always try making a PR for it and see what happens, though since the compiler uses PGO I doubt it'll show any improvement in the compiler perf suite.

8 Likes
3

The other thing that people are already doing here is doing this specifically for error types.

If you look in anyhow, for example:

That's marked #[cold] so if you're using anyhow then any time ? wraps an error into an anyhow::Error it's already treated as cold automatically.

Which is nice in that it's specific to that error type, since for anyhow it definitely makes sense -- anything non-cold probably shouldn't be taking backtraces and such anyhow.

3 Likes
4

For what it's worth, in time the macro that I use to avoid calling From (due to const limitations) has an explicit cold path annotation. I have measured real improvements by doing that.

1 Like