C++ inlining functions and classes help

Pages: 12
timhunderwood (7)
Hi I'm writing my first big object orientated program in C++ and I did it like this.

I have 3 classes TestMass, Galaxy and Universe each with a .cpp file and a .hh file. I have a make file that creates a universe.exe and in the process makes the 3 .o files. There are no implementations in the .hh file just class declarations and all member functions are defined outside of the definition in the .cpp files.

and it all works great...

Now I was told that my code has bits / all that are suitable for inlining. I've tried to work out roughly what this means but what I really want is to do it and see if my speed increases. But I'm getting confused...

To inline all my member functions of my classes do I have to get rid of the .cpp files and put everything in the .hh file? or can I just use the inline command next to my member functions in the .cpp file? How would I inline just some of my functions? What about the .o files ? If it's all inlined do theses still exist?

Sorry, im quite confused but I'm more interested in the syntax more than what's going on at a low level.

Thanks

Tim
moorecm (1932)
To inline a method, you have two options:

1. Move the method definition into the class definition. For example:
1
2
3
4
 
// header file
class A {
    void f() { /*...*/ }  // implicitly considered inline
};

2. Use the inline keyword and move the implementation into the header file outside of the class definition. For example:
1
2
3
4
5
6
 
// header file
class A {
    void f();
};

inline void A::f() { /*...*/ }

In either case, the definition must be in the header file to be a candidate for inlining (which is ultimately left up to the compiler). See the inclusion model and the one definition rule for more details.

EDIT: I think the best practice is to use the inline keyword at the definition not the declaration. I'm pretty sure you could use the keyword in either or even both places.
Last edited on
onur (128)
> To inline all my member functions of my classes do I have to get rid of the .cpp files and put everything in the .hh file?

You still need the .cpp file. But the definition has to be moved from the .cpp file to the header file.
You can either put them right into the class definition or put them below the class definition and add an "inline" before the method declaration in the class definition.


> How would I inline just some of my functions?
Simply just move those you want to inline and leave the others in the cpp file.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
 
// in the header
class MyClass
{
  public:
  void f() {}; // implicitly inlined since definition inside class
  void g(); // explicitly inlined trough keyword "inline" in the definition; definition needs to be in header
  void h(); // not inlined
};

inline void MyClass::g() // move this from your cpp file to the header
{

}

// in the cpp file

void MyClass::h() // leave it in the cpp file
{

}



Btw: It's finally up to the compiler what is inlined. See the "inline" keyword rather as a request or hint than a command!
Last edited on
moorecm (1932)
It's also worth mentioning that good candidates for inlining should always be very small. If the method does time consuming work or calls other non-inlined methods, it is unlikely that removing the overhead of a relatively fast function call will be beneficial.
rapidcoder (1010)
Inlining is not only removing the cost of the call. It is very often a step that allows more aggressive optimisations later - e.g. constructor elision, common subexpression elimination, dead code elimination etc.
timhunderwood (7)
Thanks this is very helpful.

So if I move a function to the header file and inline it do I completely remove its implementation in the .cpp file?

I.e. in Onur's example:

does the .cpp not mention g() at all?

also I've been taugt to put a class declaration at the top of my .cpp file as well as in the header. This seems to work so My .cpp file would be like 

1
2
3
4
5
6
7
8
9
10
11
12
 
class MyClass
{
  public:
  void f() {}; // implicitly inlined since definition inside class
  void g(); // explicitly inlined trough keyword "inline" in the definition; definition needs to be in header
  void h(); // not inlined
};

void MyClass::h() // leave it in the cpp file
{

}


is that right?

So I should mainly be inlining my get and set member functions and some other small ones?


Thanks

Tim

onur (128)
Not quite.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
#include "MyClass.h"

class MyClass
{
  public:
  void f() {}; // implicitly inlined since definition inside class
  void g(); // explicitly inlined through keyword "inline" in the definition; definition needs to be in header
  void h(); // not inlined
};

void MyClass::h() // leave it in the cpp file
{

}


You cannot redefine the class. It's already defined in the header, so simply include the header.
(Don't forget the include gaurds in the header file!)
onur (128)
maybe you'll also want to take a look at that:
http://www.cplusplus.com/forum/articles/20600/
moorecm (1932)
also I've been taugt to put a class declaration at the top of my .cpp file as well as in the header. This seems to work so My .cpp file would be like


Don't do that; use the include directive. Think of #include's as inserting a text file verbatum by the preprocessor. (The end result is that it is in both places.)
timhunderwood (7)
ok! It's almost working but I'm getting a strange error I don't understand. (firstly I removed all my redefinitions of the classes in the .cpp file and used #includes.

I then moved my simple method set Position to the testMass.hh file and removed it from the .cpp file
1
2
3
4
 
inline void CTestMass::setPosition(const Vec2& newPosition)
{
	position = newPosition;
}


and it worked (and actually ran quite a bit faster with just that small change
.

Then I tried to move another heavily used small method get Position

1
2
3
4
 
inline const Vec2 & CTestMass::getPosition()
{
	return position;
}


BUT I got an error when trying to make 
1
2
3
4
5
6
7
8
9
10
11
12
13
 
g++ -O2 -g -pedantic -Wall  -I/ux/physics/part_2/c++ -I/usr/include/ -I/usr/include  -c -o testMass.o testMass.cpp
g++ -O2 -g -pedantic -Wall  -I/ux/physics/part_2/c++ -I/usr/include/ -I/usr/include  -o universeClass.exe  universeClass.o galaxy.o testMass.o /ux/physics/part_2/c++/cavlib/cavlib.a -lgsl -lgslcblas -lfftw3
universeClass.o: In function `circularMotionPosition(CGalaxy&, double, double)':
/home/th347/ETP/Computing/project/code/universeClass.cpp:534: undefined reference to `CTestMass::getPosition()'
universeClass.o: In function `CUniverse::initialiseSimulation_Statistics()':
/home/th347/ETP/Computing/project/code/universeClass.cpp:367: undefined reference to `CTestMass::getPosition()'
/home/th347/ETP/Computing/project/code/universeClass.cpp:367: undefined reference to `CTestMass::getPosition()'
/home/th347/ETP/Computing/project/code/universeClass.cpp:368: undefined reference to `CTestMass::getPosition()'
/home/th347/ETP/Computing/project/code/universeClass.cpp:368: undefined reference to `CTestMass::getPosition()'
universeClass.o:/home/th347/ETP/Computing/project/code/universeClass.cpp:414: more undefined references to `CTestMass::getPosition()' follow
collect2: ld returned 1 exit status
make: *** [universeClass.exe] Error 1


It doesn't seem to able to see the function CTestMass::getPosition() when its inlined in the header file although it finds it fine when it's in the .cpp file. SetPosition() is never called directly in universeClass.cpp perhaps that has something to do with it?

It seems to give this error for all my get functions but not my set functions... Is there some syntax I'm missing? The relative bit of my make file is

universeClass.exe : universeClass.o galaxy.o testMass.o
$(CC) $(INCLUDE) -o $@ universeClass.o galaxy.o testMass.o $(LIBS)


Thanks very much for all the help so far!
Last edited on
hanst99 (2869)
Just a short comment on inlining: The inline keyword is just a suggestion to the compiler. Functions that are declared as inline may be compiled as normal functions, and normal functions may be compiled as inline functions - the compiler decides on the optimizations.


and it worked (and actually ran quite a bit faster with just that small change


I can't imagine that inlining a single setter method with no logic behind it would make any real difference on runtime. Chances are your compiler inlined that function either way.

Disch (13742)
I don't think the compiler can inline functions that aren't declared as inline. At least not across different cpp files. Or if it somehow does, it doesn't optimize the same way it would if the functions were inlined.

If you think about it... I mean the compiler optimizes the code and decides whether to inline stuff. But the compiler only works on one file at a time. It can't inline a function defined in b.cpp when it's compiling a.cpp.

It isn't until linktime when the object files are tied together, and by then it's too late to modify/optimize the compiled code (other than removing bits that are unused).


I'm not 100% sure, of course, so I might be wrong. But it just seems really unlikely to me.
Last edited on
timhunderwood (7)
by "quite a bit faster" I meant a 0.2 second improvement on a 7 second run time. It's a function that gets called 10,000s of times.

Does anyone have any idea what might be causing my error above? I want to try and inline all suitable functions and try and get an overall runtime improvement.

moorecm (1932)
I don't think the compiler can inline functions that aren't declared as inline. At least not across different cpp files. Or if it somehow does, it doesn't optimize the same way it would if the functions were inlined.

If you think about it... I mean the compiler optimizes the code and decides whether to inline stuff. But the compiler only works on one file at a time. It can't inline a function defined in b.cpp when it's compiling a.cpp.

It isn't until linktime when the object files are tied together, and by then it's too late to modify/optimize the compiled code (other than removing bits that are unused).


I'm not 100% sure, of course, so I might be wrong. But it just seems really unlikely to me.


This is exactly how I picture it, as well. I think it's safe to assume because, for example, g++ can be called from the command line separately for each of the objects and later linked--the compiler would have no way to know they are related calls.

It also explains why inline functions are defined in header files.
moorecm (1932)
Does anyone have any idea what might be causing my error above? I want to try and inline all suitable functions and try and get an overall runtime improvement.


Post the code you have now verbatim so we can take a good look at it.
hanst99 (2869)
I mean the keyword is a suggestion, of course the function still has to be in the same compilation unit.
timhunderwood (7)
I closed everything and opened it again and recompiled and it worked no problems. Not complaining.

Thanks for all the help I'll let you know how much it improved the run time

cheers
timhunderwood (7)
Ok (its working) inlining improved a 4.4 second time to a 4.2 second time.

Now the situation is that in some of my .cpp files there is almost no code which seems a bit silly. So I though I might as well put it all in the .hh file.

Firstly is that ok?/a good idea? / good style?

Secondly:

it didn't work

I got this error

make universeClass.exe
g++ -O2 -g -pedantic -Wall -I/ux/physics/part_2/c++ -I/usr/include/ -I/usr/include -c -o universeClass.o universeClass.cpp
make: *** No rule to make target `galaxy.o', needed by `universeClass.exe'. Stop.

which is definitely because of something in my make file. Unfortunately I really don't know a lot about make files (some one helped me write it). What do I need to change?

MAKEFILE
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
 
# Define the source code modules for the library:

SOURCE = test1.cc

default: execs 

.PHONY: default execs output clean plots all

.SUFFIXES: .cc .hh .o .gp .cpp ,hpp

CC         = g++ -O2 -g -pedantic -Wall 
INCLUDE    = -I$(CAV_INC) -I$(GSL_INC) -I$(FFTW_INC) 
LIBS       = $(CAV_LIB) $(GSL_LIB) $(FFTW_LIB)
RM         = /bin/rm -f

universe.exe : universe.o galaxy.o testMass.o
	$(CC) $(INCLUDE) -o $@  universe.o galaxy.o testMass.o $(LIBS)

universeClass.exe : universeClass.o galaxy.o testMass.o
	$(CC) $(INCLUDE) -o $@  universeClass.o galaxy.o testMass.o $(LIBS)
	
galaxy.exe : galaxy.o testMass.o
	$(CC) $(INCLUDE) -o $@  galaxy.o testMass.o $(LIBS)
	
%.o : %.cc
	$(CC) $(INCLUDE) -c -o $@ $<

%.o : %.cpp
	$(CC) $(INCLUDE) -c -o $@ $<

%.exe : %.cc
	$(CC) $(INCLUDE) -o $@ $< $(LIBS)

%.exe : %.cpp
	$(CC) $(INCLUDE) -o $@ $< $(LIBS)

%: %.cc
	$(CC) $(INCLUDE) -o $@ $< $(LIBS)

%: %.cpp
	$(CC) $(INCLUDE) -o $@ $< $(LIBS)

%.out: %.exe
	./$<  $(DUMMYARGS) >  ./$@

execs: $(SOURCE:.cc=.exe) 

output: $(SOURCE:.cc=.out) 

clean:
	$(RM) $(SOURCE:.cc=.o) $(SOURCE:.cc=.exe) 

cleanout:
	$(RM) $(SOURCE:.cc=.out)



The truth is I'm a python programmer and so I'm not used to having to do all this compiling/linking/header file stuff...

Help Appreciated

moorecm (1932)
The make file is saying, "How do I make galaxy.o? I see that universeClass.exe wants to link with it."

From the makefile, it looks like you need a galaxy.cc or galaxy.cpp. Is galaxy just a header now? If so, remove galaxy.o from lines 17, 20, and 23; and make galaxy.o galaxy.hpp (or whatever) on lines 16, 19, and 22.
timhunderwood (7)
Thanks that worked.

Is it better to have everything in a .hh file ?

or is it more efficient to have .hh .cpp and .o ? (speed is of importance)

Pages: 12