Fix json output in the self-profiler#56091
Conversation
Fix missing ',' array element separators and convert NaN's to 0.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
| if total > 0 { | ||
| ((hits as f32) / (total as f32)) * 100.0 | ||
| } else { | ||
| 0.0 |
There was a problem hiding this comment.
You could use Laplace smoothing instead - (1 + hits) / (2 + total).
It's really simple, but still useful when you want to count almost any kind of interesting/all fractions.
There was a problem hiding this comment.
I'm intrigued but I don't think I completely understand the idea. In the case where the category has no data hits=0 and total=0 so wouldn't this result in (1 + 0) / (2 + 0) => 1/2 => 0.5 aka 50% cache hits when there were no hits or attempts at all?
There was a problem hiding this comment.
Yeah, if there's zero prior knowledge then the cache hit probability is 50% - it's either hit or not :)
If you have some better prior knowledge than that, then you can tweak the α parameter in (α + hits)/(2α + total).
Either way you won't have to divide by zero.
There was a problem hiding this comment.
It just seems like a bug to me that if there's no data, cache hit ratio would be 50%. If it's ok with you, I'd prefer to leave the code as is. :)
|
@bors r+ |
|
📌 Commit c7dc868 has been approved by |
… r=petrochenkov Fix json output in the self-profiler Fix missing ',' array element separators and convert NaN's to 0. cc @Mark-Simulacrum
Rollup of 14 pull requests Successful merges: - #55767 (Disable some pretty-printers when gdb is rust-enabled) - #55838 (Fix #[cfg] for step impl on ranges) - #55869 (Add std::iter::unfold) - #55945 (Ensure that the argument to `static_assert` is a `bool`) - #56022 (When popping in CTFE, perform validation before jumping to next statement to have a better span for the error) - #56048 (Add rustc_codegen_ssa to sysroot) - #56091 (Fix json output in the self-profiler) - #56097 (Fix invalid bitcast taking bool out of a union represented as a scalar) - #56116 (ci: Download clang/lldb from tarballs) - #56120 (Add unstable Literal::subspan().) - #56154 (Pass additional linker flags when targeting Fuchsia) - #56162 (std::str Adapt documentation to reality) - #56163 ([master] Backport 1.30.1 release notes) - #56168 (Fix the tracking issue for hash_raw_entry) Failed merges: r? @ghost
Fix missing ',' array element separators and convert NaN's to 0.
cc @Mark-Simulacrum