mimalloc icon indicating copy to clipboard operation
mimalloc copied to clipboard

Refactoring bug, searches stats field

Open bazineta opened this issue 2 months ago • 1 comments

There appears to be bug in the 'searches' stat as a result of https://github.com/microsoft/mimalloc/commit/56aba086eaea3893e411f51bc9615663763592c7.

It's here: https://github.com/microsoft/mimalloc/blob/09a27098aa6e9286518bd9c74e6ffa7199c3f04e/src/stats.c#L238

const int64_t avg_tens = (stat->total == 0 ? 0 : (stat->total*10 / stat->total));

If total is zero, then avg_tens is 0. If total is non-zero, avg_tens is 10; we're multiplying total by 10 and dividing it by itself.

Prior to the refactoring in that commit, the searches field was a tuple, containing a count and a total, and the math used to be:

const int64_t avg_tens = (stat->count == 0 ? 0 : (stat->total*10 / stat->count));

@daanx Not sure what the correct course of action is here -- should this be divided by some other field now?

bazineta avatar Oct 04 '25 16:10 bazineta

@daanx I believe the point of this particular counter was to facilitate a running average, i.e., stuff per iteration. To that end, I've experimented with an additional type in mimalloc-stats.h:

typedef struct mi_stat_average_s {      
  int64_t total;
  int64_t count;
} mi_stat_average_t;

And added the requisite functions and macros; the average computation then becomes straightforward, basically, returning to as it was before the refactoring.

Obviously, that's an incompatible change to the stats structure, so it'd have to be revved to 3. Other than that, the changes are an easy lift and seem to test properly.

bazineta avatar Oct 04 '25 23:10 bazineta