microsoft / microsoft/mimalloc

Observation: purged and reset statistics types

Open
#1,151 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
13.4k
Forks
1.2k
Avg merge
4d 45m
Merged PRs (30d)
13

Description

The answer to this is probably _"we know, it's for ABI compatibility"_, but just in case:

The statistics `purged` and `reset` variables are presently `mi_stat_count_t`, i.e., a 3-tuple of peak, current, and total.

https://github.com/microsoft/mimalloc/blob/09a27098aa6e9286518bd9c74e6ffa7199c3f04e/include/mimalloc-stats.h#L32

However, use of these variables is just as an increasing counter, i.e., their usage actually models a `mi_stat_counter_t`. They're only ever increased, in these two locations:

https://github.com/microsoft/mimalloc/blob/09a27098aa6e9286518bd9c74e6ffa7199c3f04e/src/os.c#L516
https://github.com/microsoft/mimalloc/blob/09a27098aa6e9286518bd9c74e6ffa7199c3f04e/src/os.c#L548

They're only read here:

https://github.com/microsoft/mimalloc/blob/09a27098aa6e9286518bd9c74e6ffa7199c3f04e/src/stats.c#L333

And that's literally the only use of `mi_stat_peak_print()`; it's just to extract the `peak` value from these, the `total` and `current` fields are never read, and those fields will always contain the same value. It seems these could be changed to be `mi_stat_counter_t`.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the type definitions in include/mimalloc-stats.h, then inspect the two increments in src/os.c and the read in src/stats.c. Check the ABI-compatibility concern before changing the types; done means the representation matches the counter usage without breaking the existing statistics output.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
performance
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.