Sign in to follow this  
TheComet

Getter + dirty flag + "mutable"

Recommended Posts

I've run into this problem a few times in my own code and in code from open source projects and would like to hear your op-onyos about it. It basically looks like this:

class Foo
{
    float cachedResult_;
    bool dirty_;
public:
    void doSomethingThatChangesResult() { dirty_ = true; /* ...*/ }
    float getResult()
    {
        if (dirty_)
            cachedResult_ = doComplexCalculation();
        return cachedResult_;
    }
};

The problem is that you cannot use Foo::getResult in any other methods that are const. As a client of this class, you don't necessarily care about whether the result is cached or not (or maybe you should be caring, and that is the core of this apparent issue?)

 

Is it acceptable to make dirty_ and cachedResult_ mutable in exchange for making getResult() const?

Edited by TheComet

Share this post


Link to post
Share on other sites
4 minutes ago, TheComet said:

Is it acceptable to make dirty_ and cachedResult_ mutable in exchange for making getResult() const?

Yes, as far as I'm aware this is exactly what mutable is for. The dirty-flag/caching are just implementation-details that the client doesn't care about, so getResult() can be used identically as if doSomethingThatChangesResult just wrote the result directly. Its bettern than to mark would-be const-functions non-const, as this could easily break const-correctness in some major ways.

Share this post


Link to post
Share on other sites
10 minutes ago, TheComet said:

I've run into this problem a few times in my own code and in code from open source projects and would like to hear your op-onyos about it. It basically looks like this:


class Foo
{
    float cachedResult_;
    bool dirty_;
public:
    void doSomethingThatChangesResult() { dirty_ = true; /* ...*/ }
    float getResult()
    {
        if (dirty_)
            cachedResult_ = doComplexCalculation();
        return cachedResult_;
    }
};

The problem is that you cannot use Foo::getResult in any other methods that are const. As a client of this class, you don't necessarily care about whether the result is cached or not (or maybe you should be caring, and that is the core of this apparent issue?)

 

Is it acceptable to make dirty_ and cachedResult_ mutable in exchange for making getResult() const?

And what about calling doComplexCalculation in doSomethingThatChangesResult ?

Since it seems you have access and can modify the original code, it might be less dirty to do so. You will then be able to remove the dirty flag and the necessity to have the mutable qualifier. Which, IMHO, is far better than adding mutable just to add it. The cost is that the complex calculation will have to be done in doSomethingThatChangesResult .

Share this post


Link to post
Share on other sites

@_Silence_ that would be less than optimal and defeating the purpose of the dirty flag in the first place. In practice, doSomethingThatChangesResult() can be called anywhere from 2 to thousands of times before getResult() is called. With your suggestion, the time consuming calculation would needlessly be performed thousands of times instead of just once.

Share this post


Link to post
Share on other sites

While folks are correct that this is the poster child use case of mutable, keep in mind that the contract for mutable has changed in C++ 11 if this code is ever to be multi-threaded.  As of C++ 11, the contract for mutable now also includes a statement of thread safety.  A use case such as this in a multi-threaded engine will likely fail pretty miserably and you need to protect the cacheResult_ value.

I'm only pointing this out 'in case' you intend to multi thread any of this code, if not it doesn't impact you..

Share this post


Link to post
Share on other sites
29 minutes ago, TheComet said:

@_Silence_ that would be less than optimal and defeating the purpose of the dirty flag in the first place. In practice, doSomethingThatChangesResult() can be called anywhere from 2 to thousands of times before getResult() is called. With your suggestion, the time consuming calculation would needlessly be performed thousands of times instead of just once.

OK. But I could not guess that at first glance :) And at the light of what you revealed, for sure what I said is not a good solution at all.

Share this post


Link to post
Share on other sites
9 hours ago, TheComet said:

(or maybe you should be caring, and that is the core of this apparent issue?)

Yep, this is bad, assuming we're talking about gamedev / realtime.

The resource cost of a function is part of it's interface / public contract. 

Having a public contract that says "sometimes this takes nanoseconds but other times it takes milliseconds" is smelly. Having such a contract be implicit / hidden is just evil.

I've worked with an engine in the past that shall not be named where everything was written like this. Best case performance was amazing, worst case was unshippable, and average case had such high deviation that we had to rewrite huge chunks of the engine in a hurry.

tl;dr - I'm burned :D

Share this post


Link to post
Share on other sites
9 hours ago, TheComet said:

@_Silence_ that would be less than optimal and defeating the purpose of the dirty flag in the first place. In practice, doSomethingThatChangesResult() can be called anywhere from 2 to thousands of times before getResult() is called. With your suggestion, the time consuming calculation would needlessly be performed thousands of times instead of just once.

If getResult is only called once, why even bother caching the value?

Share this post


Link to post
Share on other sites

My general take on this is that such cases are softly violating single-responsibility and are code smells. 20+ years of coding and I've legit needed mutable maybe 3 times, I felt "dirty" about each of them, and ultimately all the cases I can remember were later removed during refactoring that left the code smaller and faster.

I look at it like this: You have an object whose job is to receive and accumulate mutations. You have an object whose job is to provide the post-processed aggregate of those mutations. You have not yet illustrated that these must be the _same_ object. I don't know your use case so maybe they do need to be, but I'd be willing to bet that the code can be easily restructured to avoid it.

Such caching mechanisms are necessary when you have an unpredictable pattern of mutation and read-back. That honestly just shouldn't be something you need all that often in well-structured and efficient software. Execute all your mutations in one pass. Transform the data into its efficient-for-use format. Then execute the passes that require that computed state. Look at it as a pipeline of data transformations. Graphics, physics, AI, all of it can be written that way for pretty much any game from Tetris to Destiny.

When a caching layer really is needed (e.g. in front of IO) then build the cache as a second layer (a separate object). In place of a dirty flag, revision numbers/counters work a lot better here too, in particular because you then no longer need to mutate a dirty flag back to a clean state; your cache can just compare its last revision number with the source's current revision to know if it should update and never needs to modify the source in any way. 

Share this post


Link to post
Share on other sites
19 hours ago, SeanMiddleditch said:

My general take on this is that such cases are softly violating single-responsibility and are code smells. 20+ years of coding and I've legit needed mutable maybe 3 times, I felt "dirty" about each of them, and ultimately all the cases I can remember were later removed during refactoring that left the code smaller and faster.

I look at it like this: You have an object whose job is to receive and accumulate mutations. You have an object whose job is to provide the post-processed aggregate of those mutations. You have not yet illustrated that these must be the _same_ object. I don't know your use case so maybe they do need to be, but I'd be willing to bet that the code can be easily restructured to avoid it.

Such caching mechanisms are necessary when you have an unpredictable pattern of mutation and read-back. That honestly just shouldn't be something you need all that often in well-structured and efficient software. Execute all your mutations in one pass. Transform the data into its efficient-for-use format. Then execute the passes that require that computed state. Look at it as a pipeline of data transformations. Graphics, physics, AI, all of it can be written that way for pretty much any game from Tetris to Destiny.

When a caching layer really is needed (e.g. in front of IO) then build the cache as a second layer (a separate object). In place of a dirty flag, revision numbers/counters work a lot better here too, in particular because you then no longer need to mutate a dirty flag back to a clean state; your cache can just compare its last revision number with the source's current revision to know if it should update and never needs to modify the source in any way. 

Agreed, and when combined with the decorator pattern, the rest of your code is none the wiser and can remain the same.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this