
John Carmack on Inlined Code (2014) - tosh
http://number-none.com/blow/john_carmack_on_inlined_code.html
======
m0zg
My personal rule is: if a simple (<=10LOC) function is used fewer than 3 times
in code, it's not really a function, it should be inlined. Only noobs avoid
copy&paste at all costs. There's time and place for it. Consider that code is
written once but read multiple times, and it's a lot harder to read it than to
write it, in particular if there are endless chains of function calls, as you
commonly see in e.g. Java and the like.

Edit: I don't even care about the downvotes. This is hard won wisdom. If you
don't see it, that's your problem, not mine.

~~~
simias
No need to be adversarial, we're talking about coding styles.

I actually sometimes write very short helper functions that are actually meant
to improve readability by abstracting some details and making the code more
self-documenting.

For instance I have this code in my current application (a device driver):

    
    
        static unsigned get_machine_id(struct manager *mgr)
        {
            u32 conf = readl(mgr->regs + OFF_MACHINE_ID);
        
            return conf & 0xff;
        }
    

It's called twice in the entire code. By you rule I ought to manually inline
it but I think it'd actually make the code worse, not better. It's always a
matter of balance, writing good code is an art.

>Only noobs avoid copy&paste at all costs

Only noobs rely on rules of thumb indiscriminately.

~~~
m0zg
Where did I say anything about relying on anything "indiscriminately"?
Avoiding repetition at all cost is definitely a mistake. Avoiding repetition
where it's justifiable could be a sensible choice. Though in your case I'd
just say:

    
    
      u32 machine_id = readl(mgr->regs + OFF_MACHINE_ID) & 0xff;
    

and be done with it.

~~~
Gibbon1

       #define machine_id() (readl(mgr->regs + OFF_MACHINE_ID) & 0xff)

~~~
atq2119
Please no. C-style macros are too leaky.

------
lazyjones
One has to keep in mind that these thoughts are very language- and
application-dependent. In some other languages, programming environments (IDE,
static analyzers) or even fields (i.e. less performance-critical) priorities
change. Generally, less code is better. Testable functions are better than
copy/pasted inline code. Readability is important. Thread-safety is important
(if your functions mutate some global state, good luck with that, whether
inlined or not...). But it's also hard to argue against the C++ and game
programming god himself.

As stated in the beginning of the article, these thoughts were from 2007 (not
2014).

~~~
latch
I think you're missing his point; you're on two sides of the same coin. Your
both saying "it depends", but he's taking the difficult route and highlighting
the benefits of the non-dogmatic approach. I think why you should use a
function is obvious to most of us. But an analysis of when/why you shouldn't,
with concrete list of things to look out for, is more significant. Also, I'm
not sure that his general goal (improved productivity, fewer bugs in large
codebases) isn't a priority in all languages, environment and fields. I also
think it's hard to argue that his specific recommendations aren't globally
applicable, especially when he uses the word "consider" in many of them. Is
there really a language/env/field where you shouldn't consider inlining a
function that's only ever called once?

Aside, but can we all agree that this is what good technical leadership looks
like? It's not some arbitrary law passed down about superficial stuff, it's a
thought provoking _discussion_ about code quality. It specifically doesn't
argue for a one-size-fits-all appraoch, but rather encourages people to
consider the situation on a case by case basis. It also includes a concrete
list of actions.

~~~
Nursie
> Is there really a language/env/field where you shouldn't consider inlining a
> function that's only ever called once?

Readability IMHO.

Contrast -

    
    
      // This function does three high level things
      // doHighlevelThing3 is only called once
      func () {
        doHighlevelThing1()
        doHighlevelThing2()
        doHighlevelThing3()
      }
    
      func doHighlevelThing3 () {
        doLowlevelStuff1()
        doLowlevelStuff2()
        doLowlevelStuff3()
        doLowlevelStuff4()
        doSomeBitshifting()
        writeSomeMemoryLocations()
        doLowlevelStuff5()
        doLowlevelStuff6()
      }
    

With -

    
    
      // This function does three high level things
      func () {
        doHighlevelThing1()
        doHighlevelThing2()
        doLowlevelStuff1()
        doLowlevelStuff2()
        doLowlevelStuff3()
        doLowlevelStuff4()
        doSomeBitshifting()
        writeSomeMemoryLocations()
        doLowlevelStuff5()
        doLowlevelStuff6()
     }
    

To me, the second one loses the readability of the first. There are three main
actions func takes, but in the second example we lose the clarity on that. If
we bring in all three HighLevel funcs into the low level one, we can end up
with something hard to follow and digest, and the intent gets lost.

Whether that gets inlined for performance reasons is entirely up to the
compiler, IMHO, and if I have very strong feelings I can probably hint to the
compiler (making it static in C, private in Java, whatever mechanism in other
languages) that it can do what it likes in terms of optimisation.

~~~
dagw
_To me, the second one loses the readability of the first._

The second one makes it clear that this function changes the state of
something by writing to certain memory locations, the first one doesn't. To me
that makes the second one more readable.

~~~
Nursie
> The second one makes it clear that this function changes the state of
> something by writing to certain memory locations

Seriously? So all state changes need to be made in 'main'?

Pretend there was no explicit state change in there, does that change your
mind?

What if doHighlevelThing3 is actually called "changeTheThing", and the actual
write to the memory location is simply the low-level implementation of that?

That's a really weird objection (to me, I acknowledge this is all style and I
am opinionated).

~~~
latch
Carmack has been consistent for a long time that state changes are a major
source of bugs. It's true that video games might be more susceptible to this.

I think all things considered, readability should generally take a back seat
to correctness. I mean, the code is readable, yay!, but the game crashes
randomly...not so yay...

Also, he does offer a suggestion to help address your concern:

"Using large comment blocks inside the major function to delimit the minor
functions is a good idea for quick scanning, and often enclosing it in a bare
braced section to scope the local variables and allow editor collapsing of the
section is useful."

Finally, note that he's only suggestion that you CONSIDER it, which was my
original point. It's hard to say his suggestions are wrong, when his
suggestions are merely that you consider it.

~~~
Nursie
> I think all things considered, readability should generally take a back seat
> to correctness.

I think that lack of readability is a sure path to lack of correctness,
personally. Carmack may have had the luxury of working with consistently great
engineers... I do not :)

I'm not saying his suggestions are wrong, merely that there are times when
encapsulation makes sense. Code can be readable and correct.

------
taneq
I think it's interesting how, as we push more into "this code absolutely must
be correct" territory, code starts looking more and more like PLC logic than
like high level code. Run everything every time you go through the main loop,
don't overwrite values, calculate your results by choosing which intermediate
values to use.

------
ryandrake
John's observation, that having state-mutating functions that get called only
once is actually less readable and more bug-prone, flies in the face of many
companies' coding style guidelines that place an arbitrary upper limit on the
size of functions. So you have engineers needlessly refactoring 250-line
BigFunction() into:

    
    
        BigFunction()
        {
            LittleFunction1(); // 50 lines
            LittleFunction2(); // 50 lines
            LittleFunction3(); // 50 lines
            LittleFunction4(); // 50 lines
            LittleFunction5(); // 50 lines
        }
    

Which complies with the ill-advised policy, but is often not any more
readable, and could be more bug prone. Does LittleFunction4() change some
state variable that LittleFunction5() depends on? Who knows, better go find
the source and check it. Yuck!

~~~
theptip
If you named them like that, then sure that's less readable. But I feel
there's a lot of straw man arguments in this thread; the main reason it's
generally considered good practice to split up big functions is that humans
aren't good at keeping a lot of context in their heads, so if you can provide
a higher level abstraction then you can make it easier to follow.

If we pick a more realistic

    
    
        RenderThePage()
        {
            FetchStateFromBackend()
            ProcessState()
            RenderTemplate()
            ReturnHTMLToUser()
        }
    

I would find that a lot easier to follow than jamming all of that code into a
single massive function that did all those different things. (Not to mention,
I'd find it easier to test all those things in isolation, too).

To take a completely contrarian position to that being put forth in this
thread, I often find it clearer to pull a single hard-to-follow line of code
into a function, just to encapsulate it behind a simpler named interface.

~~~
majewsky
Anecdata: Years ago, I refactored some code written by an inexperienced
programmer. It was a multiple-thousand-line Perl CGI script where everything
was on the top scope, i.e. there were not really any functions (except for a
few one-liners). When this was called out by the lead architect, he improved
the script by splitting everything into two functions:

    
    
      ($a, $b, $c, $d, ..., $x, $y, $z) = GetData();
      PrintResult($a, $b, $c, $d, ..., $x, $y, $z);
    

The variables actually had reasonable names, but there were at least 20 of
them. He just splitted the code at some random point and all variables that
needed to be used in the second half became return values and arguments,
respectively.

~~~
lmm
I'd argue that even that can be a small step in the right direction if there
were more than 20 variables at the start - at least now you have an upper
bound on how much state from the top half of the code can be being used in the
bottom half of the code.

------
p1necone
When I've done gamedev stuff in the past I have used the "single big function,
collapsible scoped blocks rather than calling separate functions" pattern a
fair bit. I do find it makes the code clearer, especially in the root level
game loop or world state setup stuff where the code reads (or at least should
read imo) a lot like a flat script.

~~~
p1necone
I feel like it made up for the overengineered bits that read like enterprise
java instead :)

------
alanwreath
It is just me or does anyone else appreciate the tone of this letter? I've
dealt with many a senior dev and architect that would have totally thrown a
mandate down and wouldn't have even spent the time to convince or sell the
idea. To me the mutual respect the letter demonstrates (given Carmack's role
and talent) is refreshing. I don't know Carmack on a personal level and have
never worked in any capacity that would put me in contact with him so maybe
I'm off base, but the message he wrote wasn't as cocky, line-in-the-sand, my-
way-or-the-highway, etc as some letters and messages I've read by far less
accomplished/respected individuals.

Some gems from the article were \- "I'm not going to issue any mandates, but I
want everyone to seriously consider some of these issues" \- "[what I'm
proposing is different] so I am going to try and make a clear case for it" \-
(at the very end)"Discussion?"

------
xiphias2
A fun thing is that using a function at most once has a strong relationship
with linear / unique type system.

Of course it would have been even closer to the type system if he wrote that
also variables should be used only once, but now Rust showed us how many
advantages it can have.

It would be also interesting if a function in Rust would have to be cloned to
be used multiple times (as it's an expensive operation for inlining), but of
course it would have UX disadvantages in the programming language.

~~~
SamReidHughes
(Edit: Wrong in the sense of disagreement.) It’s different, calling a function
from one place, or an enumerated set of places, than using a value once. It’s
more related to effect systems. For example, if you only load a database
config in one place, then you only connect to the database from stuff
connected to that source location, and so on up through callers, which helps
you track down how a certain class of database transaction might be performed
by your program.

~~~
xiphias2
If you look at lambda calculus where all values are encoded as functions, the
difference between using a value multiple times and using a function multiple
times goes away.

I'm not a type system expert, but I'm sure some of the theories that work on
unique types work on non-function values and function values at the same time.

~~~
SamReidHughes
We're not talking about using a function value once, we're talking about using
a function value repeatedly, from one callsite in the source code. So the
function is not getting consumed.

But you were correct. There is a sense of uniqueness that could be captured in
a linear type system. If we want the function's computation run exactly once
per frame, you might imagine a linear value created at the top of the game
loop, threaded down to the callsite where the function consumes it.
Alternatively, the function could emit the linear value, which then it gets
returned all the way back out to something which explicitly consumes it at the
end of the loop. Or maybe you encode this mechanism into the type system or as
a language feature some other way, but the point is, it's not the function
itself that gets used once -- it's an input or output parameter, combined with
some sort of access control that guarantees only the outer game loop code can
create or destroy such a value.

------
archagon
This is one of those articles that completely went in one ear and out the
other when I first ran into it, fresh out of college. A few years later, after
completing my first difficult and performance-intensive project, I saw it
again, and this time it was as if someone was reading my mind. Carmack’s
thoughts lined up point-by-point with ideas about “development-optimized”
programming I was coming around to on my own. A great bit of timeless wisdom!

(By the way, Swift is especially great for writing “Style C”:

label: do { /* code here */ }

It’s self-commenting and you can break back to label: at any time, which IIRC
will skip the rest of the block.)

------
dajonker
Refreshing to hear a point of view where it makes sense to have large
functions to guarantee sequence and avoid reusing functions. For some time I
thought it was _always_ good practice to split functions up into smaller
functions where reasonable. On the other hand I've always hated linters that
complain about method length or cyclomatic complexity, as they don't really
measure how easy it is for people to understand what is happening. After all,
code is for people, not machines.

~~~
dasmoth
Cyclomatic complexity might be a textbook example of Goodhart's law. Of course
few-paths code is easier to read, but that's often because it's doing
something that's fundamentally simpler. And refactoring just to hit some
arbitrary CC target seems _very_ unlikely to be a route to happiness.

~~~
andreareina
For the unfamiliar:

 _Goodhart 's law is an adage named after economist Charles Goodhart, which
has been phrased by Marilyn Strathern as: "When a measure becomes a target, it
ceases to be a good measure."_

[https://en.wikipedia.org/wiki/Goodhart's_law](https://en.wikipedia.org/wiki/Goodhart's_law)

------
cmroanirgo
From the article:

"To sum up:

If a function is only called from a single place, consider inlining it.

If a function is called from multiple places, see if it is possible to arrange
for the work to be done in a single place, perhaps with flags, and inline
that.

If there are multiple versions of a function, consider making a single
function with more, possibly defaulted, parameters.

If the work is close to purely functional, with few references to global
state, try to make it completely functional.

Try to use const on both parameters and functions when the function really
must be used in multiple places.

Minimize control flow complexity and "area under ifs", favoring consistent
execution paths and times over "optimally" avoiding unnecessary work."

What I found particularly interesting was: "I now strongly encourage explicit
loops for everything, and hope the compiler unrolls it properly". I suppose
looking at the generated listing code will always be the final arbiter in
whether the compiler does so or not for tiny loops.

------
notacoward
One overlooked aspect of the Gripen example is not one function vs. many, but
the fact that the control flow was expressed as a form of state machine. As
John mentions, the real problem is often that people don't fully understand
the context in which a piece of (copied) code might be called. State machines
make this more explicit, which makes expectations clearer and easier to verify
or even enforce. You can implement a state machine within one function or
across many, so it's really a bit orthogonal to the issue of inlining/copying
and I think it's a bit of a shame that the OP kind of conflates the two. The
more important lesson IMO is to manage the execution state, and how to break
that down into functions is secondary.

------
ahaferburg
The only valid counterargument to this is testability. You can't unittest a
code block that's somewhere deep inside another function. At least not in
today's languages. If you don't care about testability, a 5k LoC main() is
fine.

------
erikb
Well, it's always a trade-off. When you inline stuff that is in separate
functions you certainly increase the debuggability and readability and maybe
even the performance of that one usecase you currently think about. If your
goal is rather straightforward and your code is unlikely to be reused in high
percentages in later developments (i.e. if you are coding a game in C++) then
it might be a really good idea.

But if you work for instance on MS Word your goals will change 2-4x a year,
you will have a list of hundreds of goals, your code might be reused in MS
Excel without people even telling you, etc. In such kind of situation it is
much, MUCH more important to encapsulate everything in classes and methods and
functions (methods changing states, functions not) in a way that it can be
treated as a black box. So in that kind of situation you are rather building
lego blocks and hope that they can be connected easily enough to build the
house that the little child, which is your boss, wants. In that case your lego
blocks each need to be very testable and very connectable.

And the truth is that most of us live somewhat in the middle. We have a clear,
highest goal that needs to achieved asap, while at the same time having loads
of competing medium-priority goals that change all the time. So, while John's
insides might be awesome by itself if you haven't thought about this topic
before, please don't go full steam in that direction for a few years now. The
best result is usually a little unclear and in the middle between two
extremes.

------
seanalltogether
> A large fraction of the flaws in software development are due to programmers
> not fully understanding all the possible states their code may execute in.

I keep that quote in the back of my mind every day while working. I think it
has the largest influence on how I write code now.

------
ivanb
Long before endless Windows vs Linux debate there was "object-oriented vs
functional" debate. But even before that there was a debate on Usenet about
subroutines vs inlining in Fortran. It is weird to see this ancient debate
back.

------
__bjoernd
Am I the only one wondering how the discussion to his email thread went back
then?

------
entity345
> if you are going to make a lot of state changes, having them all happen
> inline does have advantages; you should be made constantly aware of the full
> horror of what you are doing.

That's perhaps the main issue: crucial and unexpected state change hidden in
functions.

Functions should do a single thing and should have explicit names.

We've all encountered functions with innocuous names a la "printSomething"
that actually also changed internal state by stealth...

------
ggm
Not _one_ word about Duff's device?

~~~
SamReidHughes
What does Duff's Device have do with the article?

~~~
harry8
Loop unrolling is obviously a form of inlining. Perhaps OP is considering the
case. "No backwards branches.

For a long loop no branch back is clearly silly. Is unrolling and inlining
part of it worthwhile? How does it help or hider readability, unnecessary
operation identification, (eg repeated intialisation) and bug identification?
Does it have a tension with execution time? These answers would likely be
different in 2019. SIMD intrinsics anyone because compilers can't do it well
at all. Function with a descriptive name that is called or just write it all
right there probably with a comment right where the function call would have
been and right where it would have returned..?

We're talking low level optimisation or he wouldn't be having the discussion
in terms of c++ etc. Duff is frequent touchstone is such a discussion although
not mandatory by any means.

(He said trying to get a little of such discussion going ;-)

~~~
SamReidHughes
This article is not about low level optimization.

~~~
harry8
Try to be more constructive. I think it is. I explained why I think so.

You've not explained anything, just 2 really negative posts of little
productive use to anyone. No need to even post that kind of thing, right?
Contributes nothing to a discussion. Explaining your point of view, might.

