If we used LFU we wouldn't need EventQueue's dependency, making the test slightly simpler. This change could also be used to improve the fixture, not requiring makeReplacementPolicy() anymore, allowing us to simply have:

class BaseRPStatsTest : public ::testing::Test
{
  protected:
    TestableLFU rp;
    BaseRPStatsTest() : rp(LFURPParams()) {}
};

I see 3 possible approaches to take:

• the "gem5 pattern" (i.e., this proposal, and what we currently do everywhere)
  PROS: It is simple, works; and people are used to it, so adopting it is a no-brainer.
  CONS: Breaks the single responsibility principle (SRP): now RPs take care of RP AND stats.

• decorator pattern

class StatisticsDecorator : public ReplacementPolicy {
private:
    std::shared_ptr<ReplacementPolicy> wrappedPolicy;
    Stats stats;
    
public:
    void reset(const std::shared_ptr<ReplacementData>& data) override {
        stats.resets++;
        wrapped_policy->reset(data);
    }
    // ... similar for other methods
};

PROS: Simple, maintains SRP, and allow enabling/disabling stats easily. Decorators can even be added one on top of the other to selectively add more stats. We can even add decorators to add tracing! IMO it is the ideal solution.
CONS: Additional learning curve. Has an ugly instantiation (rp = replacement_policy::StatisticsDecorator(LFU()), or something like that), but that drawback can be reduced with a Builder pattern, I think.

• observer pattern

class ReplacementPolicy {
    std::vector<ReplacementPolicyListener*> listeners;
    
    void touch(const std::shared_ptr<ReplacementData>& data) override {
        for (auto listener : listeners) listener->onTouch();
        touchImpl(data);
    }
};

class StatisticsListener : public ReplacementPolicyListener {
    void onTouch() override { stats.touches++; }
    // ...
};

PROS: allows enabling/disabling stats easily, and we can add listeners for things other than stats, like tracing, and whatever else.
CONS: it still breaks SRP, but it is more complex (yeah, I don't think this is a good solution) ;)

================================================================
All that said, I am ok with the proposed solution - so why do I bring the alternatives up, you may ask? At some point we were discussing enabling/disabling stats (maybe with @giactra ?), and these alternative patterns would be able to cover that. We have the opportunity here to start leaning into the change - but do we want to take it? I think the answer is "no, if we do this we should apply this everywhere, not only here", but at the same time I personally see the RPs as a starting point for change (the first SimObject's unit tests are RP's).

Thoughts?

Yeah... after giving it some thought, I don't think the decorator pattern would work without significant rework of the whole codebase, since it only works well for the boundaries of a function. Since we only care about enabling/disabling stats, this can be done easily through other means, like what we currently do for tracing.

Nit: we can't invalidate more than we have inserted, so we could add an assertion for that here

We should probably do the full round of checks for each of the 3 spots here. i.e./e.g., where we currently check that insertions are updated when reset is called, we should also check that the other stats remain unchanged.

This is not the way to tag files; the proper way to do is to go to each of these files and add ", tags=['gem5 stats']". Check src/base/SConscript for multiple examples. Although from the huge list of files here it looks like we haven't cleaned up the dependencies for the stats yet, so you may have some trouble doing so - I am not sure.

Nit: I had the impression that we had with_tags, but I can't recall where these are declared to look it up.

Yeah... after giving it some thought, I don't think the decorator pattern would work without significant rework of the whole codebase, since it only works well for the boundaries of a function. Since we only care about enabling/disabling stats, this can be done easily through other means, like what we currently do for tracing.