inline keyword unnecessary

[ghost]

From the cppreference.com

A function defined entirely inside a class/struct/union definition, whether it's a member function or a non-member friend function, is implicitly an inline function.

Some member functions marked as inline but it's not necessary as in description.

[trevordblack]

It isn't necessary.
But,

1. Explicit is better than implicit.
2. Some programmers may have forgotten, be unaware, of implicit inlining in class definitions
3. The compiler can choose to ignore implicit inlining (hell, it can choose to ignore explicit inlining)
4. When you're first learning, it isn't a bad idea to be reminded of which functions should be inlined at just a glance. Any modern compiler will pretty much just ignore inlining anyway, that's it's job. Marking functions explicit inline is for the programmer, not the program.

Point 2 above is something that we may want to mention. So, implicit inlining was a good issue to bring up.

[ghost]

I kindly disagree all the things you point out.
But,

1. I will not try to convince you.
2. C++ has a better notion for what is intended here, called constexpr.

[hollasch]

[Steve pulls out a wooden stool and an old pipe...] Way back in the early times, C had an auto keyword. In keeping with C's (and C++'s) freedom from logical keyword naming (see static), the auto keyword was a hint to the compiler that a particular variable would best be allocated on the stack. In contrast, the register keyword hinted to the compiler that a particular variable was used a lot, and best be stored in a register if possible.

Now, these were hints, and compilers were free to ignore the hints, but programmers expected that the compiler would mostly honor the intent.

Yeah, that's crazy, but we were young, and compilers were stupid. Eventually I learned that it's a big world out there, with a zoo of different architectures, register sets, and crazy machine considerations. “But I profiled it!”. On my computer with my compiler of my version at today's date.

I remember the day that I spent time optimizing our frequently-called Gaussian elimination matrix code, unrolling a triply-nested loop. It was a proud day, until I profiled the result and found my optimized code was 1,000x slower than the original for-loops three deep. That was the day that I learned that compilers could take advantage of CPU quirks and generate esoteric CISC instructions that perfectly fit the architecture. Later on I learned the lesson again with respect to what happens when you blow the instruction cache, and that small code is often faster than larger code, and in fact that code “optimized for space” would sometimes beat code “optimized for speed”.

I was super happy when the inline keyword came out, because I figured it would help me tell the compiler how to generate good code, by eliminating procedure call overhead. And then I learned again what that can do to your performance when that one additional wafer-thin inline function code bloats your function over the instruction page boundary and your performance goes way way down the toilet. I wrote code for the Xbox 360—a PowerPC architecture—and learned a lot about how alien some architectures can be, and how inline could frequently (and apparently randomly) destroy performance.

At this point, it's been well pounded into my thick skull that compiler writers are pretty darned smart, and I'm frequently not. It's simply not our business whether a function is small enough to be copy-pasted throughout the code or whether some random architecture on some users's computer that I've never touched has optimized calls that yield practically zero overhead due to predictive branching and other things smart hardware folk have created.

In my opinion, inline is an albatross, just as loop unrolling is, just as integer-only math is, just as the original auto and register keywords were. Seeing them in our code feels like a throwback to olde tymes, and I'd be happy to see it go. It doesn't help the reader, because they shouldn't be “looking through the code into the assembly”, and it doesn't help the compiler, because they know their jobs better than we do.

[ghost]

@hollasch After reading this it's all makes sense. Thank you very much for the reply.

I think, I miss that generation 😸 I never use auto and register keywords (in C's sense), to be honest never understand its purpose, and use cases. Whenever I heard about PowerPC or C64 stuff, it's all Greek to me 🤦‍♂️

One thing I remember about inline keyword from Meyers' famous book Effective C++. I reach out to my bookshelf and read it again, on Item 30, it's perfectly the case here. inline'ing member function.

I afraid to sound cocky when I suggest something. I'm probably the least intelligent guy about graphics in the room 😄

[trevordblack]

Sorry if I'm coming across as curmudgeonly without reason. My priorities in terms of source are 1. No code changes 2. Use C 3. Use the oldest subset of c++ 4. Use the smallest subset of new c++ A big part of the above is that I want the books to be as approachable as possible. I have serious concerns about the approachability of modern c++. And I'm really not a fan on modern c++ in general. I'd like to say that Hollasch elaborated what I was trying to go for, but no, his understanding and explanation is well beyond my own. Even if you're adamant that you're the least intelligent here, I might still fight you for the title.

…

On Mon, Aug 26, 2019, 04:27 Adem Budak ***@***.***> wrote: @hollasch <https://github.com/hollasch> After reading this it's all makes sense. Thank you very much for the reply. I think, I miss that generation 😸 I never use auto and register keywords (in C's sense), to be honest never understand its purpose, and use cases. Whenever I heard about PowerPC or C64 stuff, it's all Greek to me 🤦‍♂ One thing I remember about inline keyword from Meyers' famous book Effective C++. I reach out to my bookshelf and read it again, on Item 30, it's perfectly the case here. inline'ing member function. I afraid to sound cocky when I suggest something. I'm probably the least intelligent guy about graphics in the room 😄 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/RayTracing/InOneWeekend/issues/80?email_source=notifications&email_token=ACNZVKEFAJT7QBSLXKBIZCTQGO4ZPA5CNFSM4IPJX5G2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5ECZVI#issuecomment-524823765>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACNZVKBO6UV6SFVA7AHR6G3QGO4ZPANCNFSM4IPJX5GQ> .

[hollasch]

[Brief Steve] I'm leaning toward removing the inline keywords, as they do not add the value many think they do, are frequently untrue at the discretion of the compiler, and are not helpful in understanding the code.

That said, this is very low priority right now as we focus on the v2 milestone. Parking this for now until post v2.

[hollasch]

Interesting note. On Windows, Visual C++ compiler, release build, I moved the vec3 utility functions out to a separate vec3.cc file, and left the header with declarations only. Turns out this hit performance with a 20%–90% slowdown, so the inline variants of the functions outside the class are a significant performance boost (again, on the platform I tested). For vec3 methods, the inline keyword is unnecessary, and does not affect performance.

[hollasch]

Resolved in #226, changes in development branch.