To inline or not to inline

Question

I've been writing a few classes lately; and I was wondering whether it's bad practice, bad for performance, breaks encapsulation or whether there's anything else inherently bad with actually defining some of the smaller member functions inside a header (I did try Google!). Here's an example I have of a header I've written with a lot of this:

class Scheduler {
public:
    typedef std::list<BSubsystem*> SubsystemList;

    // Make sure the pointer to entityManager is zero on init
    // so that we can check if one has been attached in Tick()
    Scheduler() : entityManager(0) { }

    // Attaches a manager to the scheduler - used by Tick()
    void AttachEntityManager( EntityManager &em )
        { entityManager = &em; }

    // Detaches the entityManager from a scheduler.
    void DetachEntityManager()
        { entityManager = 0; }

    // Adds a subsystem to the scheduler; executed on Tick()
    void AddSubsystem( BSubsystem* s )
        { subsystemList.push_back(s); }

    // Removes the subsystem of a type given
    void RemoveSubsystem( const SubsystemTypeID& );

    // Executes all subsystems
    void Tick();

    // Destroys subsystems that are in subsystemList
    virtual ~Scheduler();
private:
    // Holds a list of all subsystems
    SubsystemList subsystemList;

    // Holds the entity manager (if attached)
    EntityManager *entityManager;
};

So, is there anything that's really wrong with inlining functions like this, or is it acceptable?

(Also, I'm not sure if this'd be more suited towards the 'code review' site)

Womb

Something to look at #146338 – Miles

Interesting; but I'm more concerned with the readability/design effects of inlining - in a header – Womb

It's good practice. Especially when the function does very little and they are nested. Since they can be optimised away. ps It looks neater if you use the inline keyword to list them after the class declaration. – Felloe

I found a good summary: #61330 – Inadequate

Answer 1

Inlining increases coupling, and increases "noise" in the class definition, making the class harder to read and understand. As a general rule, inlining should be considered as an optimization measure, and only used when the profiler says it's necessary.

There are a few exceptions: I'll always inline the virtual destructor of an abstract base class if all of the other functions are pure virtual; it seems silly to have a separate source file just for an empty destructor, and if all of the other functions are pure virtual, and there are no data members, the destructor isn't going to change without something else changing. And I'll occasionally provide inlined constructors for "structures"—classes in which all data members are public, and there are no other functions. I'm also less rigorous about avoiding inline in classes which are defined in a source file, rather than a header—the coupling issues obviously don't apply in that case.

Trishtrisha

Good answer; It just seems like an annoyance to have to write 3 extra lines (in a different file) for a 1-line function. – Womb

@Schnommus It's a bit of a bother for the author, but usually, when I'm developing a class, I'll have the both the source and the header open in different panes of my editor, so jumping between them is easy. (And I can copy/paste the function declaration from the header, delete the trailing ;, and start from there for the definition.) For the reader, I find it convenient to know that to find the implementation, I look in the source file, and never in the header. – Trishtrisha

I beg to differ with the characterization of inline functions as "noise": Seeing the one-liner functions is a form of documentation, often better than the comment. And the additional coupling introduced is very low, since changes in one-liners are almost always accompanied by changes in the data structure (private) part of the class. – Trilobite

@Trilobite Implementation details are noise for someone trying to understand the contracts the class provides. And implementation along the lines of return m_someInternalValue; tells the reader absolutely nothing about what is being returned. You still need comments to explain what the return value means. – Trishtrisha

Another argument against inlining is that changing the implementation of a single function could potentially trigger a rebuild of most of the project if the header is widely #included. For small projects, this isn't a problem, but for large projects that take hours to compile, it's a huge problem. On the other hand, if the function definition is in a regular source file, then only that one source file gets recompiled when that function is changed. It can also slow down compilation: inline functions must be recompiled every time the header is included, even if the inline functions aren't used. – Mercurous

Answer 2

All of your member functions are one-liners, so in my opinion thats acceptable. Note that inline functions may actually decrease code size (!!) because optimizing compilers increase the size of (non-inline) functions in order to make them fit into blocks.

In order to make your code more readable I would suggest to use inline definitions as follows:

class Scheduler
{
    ...

    void Scheduler::DetachEntityManager();

    ...
};


inline void Scheduler::DetachEntityManager()
{
    entityManager = 0;
}

In my opinion thats more readable.

Inadequate

clutter reduction technique ;) – Cartwheel

Answer 3

I think inlining (if I understood you right, you mean the habit of writing trivial code right into the header file, and not the compiler behaviour) aids readability by two factors:

1. It distinguishes trivial methods from non-trivial ones.
2. It makes the effect of trivial methods available at a glance, being self-documenting code.

From a design POV, it doesn't really matter. You are not going to change your inlined method without changing the subsystemList member, and a recompile is necessary in both cases. Inlining does not affect encapsulation, since the method is still a method with a public interface.

So, if the method is a dumb one-liner without a need for lengthy documentation or a conceivable need of change that does not encompass an interface change, I'd advise to go for inlining.

Trilobite

This is just wrong. Whether a function is "trivial" or not is an implementation detail, not the sort of thing you want to expose in a header. And code in the class definition itself is excess noise, making the class definition harder to read. Even when inlining is necessary for performance reasons, the best practice is to define the function outside of the class. – Trishtrisha

Answer 4

It will increase executable size and in some occasions this will lead to worse performance.

Keep in mind that an inline method requires it's source code to be visible to whoever uses it (ie. code in the header) this means that a small change in the implementation of your inlined methods will cause a recompilation on everything that uses the header where the inline method was defined.

On the other hand, it is a small performance increase, it's good for short methods that are called really frequently, since it will save you the typical overhead of calling to methods.

Inline methods are fine if you know where to use them and don't spam them.

Edit: Regarding style and encapsulation, using inline methods prevents you from using things like Pointer to implementation, forward declarations, etc.. since your code is in the header.

Cufic

In my experience, inlining one-line functions like in the example leads to smaller and faster code because it avoids parameter passing and function calls. – Durden

Answer 5

Inlining has three "drawbacks" at least:

1. inline functions are at odds with the virtual keyword (I mean conceptually, IMO, either you want a piece of code to be substituted for the function call, or you want the function call to be virtual, i.e. polymorphic; anyway, see also this for more details as to when it could make sense practically);

2. your binary code will be larger;

3. if you include the inline method in the class definition, you reveal implementation detail.

Apart from that it is plainly ok to inline methods, although it is also true that modern compilers are already sufficiently smart to inline methods on their own when it makes sense for performance. So, in a sense I think it is better to leave it to the compiler altogether...

Temper

An inline member function can be virtual, it may not be inlined when called but that's true for any inline function, virtual or not. – Solfeggio

@Charles Bailey: Thanks, I reworded the answer to better reflect what I think without incurring ambiguities. – Temper

My binaries get smaller when I code like in the question. What do I do wrong? – Durden

Answer 6

Methods inside class body are usually inline automatically. Also, inline is a suggestion and not a command. Compilers are generally smart enough to judge whether to inline a function or not.

You can refer to this similar question.

Integrate

Are the methods inside the class body automatically inline? Or are the methods that are defined in the header file inline? – Trihydric

@eeerahul, methods inside the class body are automatically inlined. In other cases you can use inline keyword. It guarantees that, compiler will generate only 1 copy of function per translation unit (i.e. .cpp file) – Integrate

Answer 7

In fact you can write all your functions in the header file, if the function is too large the compiler will automatically not inline the function. Just write the function body where you think it fits best, let the compiler decide. The inline keyword is ignored often as well, if you really insist on inlining the function use __forceinline or something similar (I think that is MS specific).

Isolationist