Hide raw perf counters behind --show-hidden, report cache miss rate

[msooseth]

compare no longer reports instructions, cycles, cache_references or cache_misses by default. They are still recorded in the JSON, and you can see them via --show-hidden. We now report cache_miss_rate (misses / references), since memory stalls dominate runtime usually. Also, it's a good metric to show in general, for example if we want to change to "flat set/map" datastructs as we have been doing

Declarations of self

• used AI
• read the diff forwards AND backwards. About 6 times.

[clonker]

some small remarks :) generally looking good though!

[clonker]

if cache_references is hidden, shouldn't we also hide cache_misses? also personally, i wouldn't hide instructions as an indicator of "did this code do more work or nah". i'd rather hide cpu_time i think.

[msooseth]

I think cache_misses is the more important of the two. Wait, I think cpu_time is more useful than instructions? Instructions can be a poor indicator of time taken, because e.g. of cache misses, multiscalar architectures, etc. Or am I misunderstanding something?

[clonker]

all i have ever used cache_misses for is to compute a miss rate cache_misses / cache_references, that's where my comment comes from. not sure what you'd derive from cache misses alone?
instructions is not really a good measure for time spent but it works alright for the amount of work I think.
cpu_time / wall_time would then inform on the actual time spent.

such is my understanding but i might be wrong!

[msooseth]

I think you may be confusing cpu_time -- it includes the CPU waiting. So cpu_time / wall_time is only going to tell you about IO time that the CPU is waiting (and other kernel things, but mostly IO). So I find that quite useless? Or are you interested in IO?

I find cpu_instructions very misleading. I can make it 4x and make the time 1/2 and I can make it 1/2 and make the time 4x. I think it will mislead people when they say it went up by, say, 40% when the time goes down by 20%. I want that PR to be merged, because 20% less time is amazing. Maybe you&I understand cpu_instructions, but I fear many don't. I'd be very cautious about including it in any comparison. I am already terrified of having a 40-message-long discussion about it with someone who doesn't quite understand it... And I don't have a week to waste on that discussion 😢

cache_misses / cache_references -- we can do that, OK, will show that. However, it can also lead to misunderstandings. Are you sure you want that?

If you really want all of these, you can have it, but I will be less and less interested in any discussion regarding these numbers, because, I have no energy to discuss at length about cpu_instructions going up. I'd rather not merge a 20% speedup if that means I need to argue for a week.

[clonker]

I was not suggesting we include cpu_time / wall_time as a quantity. It was meant as cpu_time slash wall_time, not division, i think we misunderstood each other here :).

as for cpu_instructions, i kind of doubt that wed have it to extremes as you outlined with solidity. afaik rustc-perf and the llvm compile time tracker thing use it as primary metric even because it has a lower signal to noise ratio. it could stay hidden as non-default output.

in any case, i am okay with not having stuff in the output and we add it if we find we need it down the line!

[msooseth]

it's now

HIDDEN = {"cycles", "instructions", "cache_references", "cache_misses"}