0ct0pu5/wesnoth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Remove use of __builtin_expect when building with GCC

Browse source 
__builtin_expect is something that should only be used if you're very sure it will result in
an optimization (see http://blog.man7.org/2012/10/how-much-do-builtinexpect-likely-and.html).

This code was added back in 2008. It seems it was part of an effort to get the preprocessor
functioning faster. It looks like mordante played some role in the decision of where to use
the LIKELY and UNLIKELY macros, so it's possible it had some use at the time (he's a rather
experienced programmer).

However, it's better to let the compiler automatically optimize code itself. I haven't done
any profiling on the preprocessor speed with or without __builtin_expect, but it doesn't seem
worth keeping this around and having to test every so often whether it's still useful or has
become a performance hit instead.

It's also worth noting that I myself noticed what seems a perceptible performance improvement
in the game (again, not backed by profiling) when I switched from TDM GCC to MSVC 2017. IIRC,
I'm using at least quick LTO for my builds. Apples and oranges, yes, but it also proves there
are likely various compiler options (such as LTO) which could improve performance on their own,
and better, than trying to point the program down branch paths ourselves.
This commit is contained in:
Charles Dang 2018-01-21 13:52:16 +11:00
parent ba5ddc4bcb
commit eb52503934
3 changed files with 9 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
src/buffered_istream.hpp
View file

@ -62,7 +62,7 @@ public:
{
fill_buffer();
if(UNLIKELY(eof_)) {
if(eof_) {
return EOF;
} else {
/*
@ -89,7 +89,7 @@ public:
{
fill_buffer();
if(UNLIKELY(eof_)) {
if(eof_) {
return EOF;
} else {
/* See get() */
@ -155,14 +155,14 @@ private:
*/
void fill_buffer()
{
if(UNLIKELY(buffer_offset_ >= buffer_size_)) {
if(buffer_offset_ >= buffer_size_) {
/*
* This does not only test for the EOF, but also makes sure the
* data is available in the buffer. Without it readsome will read
* nothing, after its first call, even if the EOF has not been
* reached.
*/
if(UNLIKELY(stream_.rdbuf()->sgetc() == EOF)) {
if(stream_.rdbuf()->sgetc() == EOF) {
eof_ = true;
} else {
buffer_offset_ = 0;

10
src/serialization/tokenizer.hpp
View file

@ -95,7 +95,7 @@ private:
void next_char()
{
if (UNLIKELY(current_ == '\n'))
if (current_ == '\n')
++lineno_;
next_char_fast();
}
@ -104,15 +104,15 @@ private:
{
do {
current_ = in_.get();
} while (UNLIKELY(current_ == '\r'));
} while (current_ == '\r');
#if 0
/// @todo disabled untill campaign server is fixed
if(LIKELY(in_.good())) {
if(in_.good()) {
current_ = in_.get();
if (UNLIKELY(current_ == '\r'))
if (current_ == '\r')
{
// we assume that there is only one '\r'
if(LIKELY(in_.good())) {
if(in_.good()) {
current_ = in_.get();
} else {
current_ = EOF;

8
src/utils/general.hpp
View file

@ -18,14 +18,6 @@
#include <algorithm>
#include <cctype>
#ifdef __GNUC__
#define LIKELY(a) __builtin_expect((a),1) // Tells GCC to optimize code so that if is likely to happen
#define UNLIKELY(a) __builtin_expect((a),0) // Tells GCC to optimize code so that if is unlikely to happen
#else
#define LIKELY(a) a
#define UNLIKELY(a) a
#endif
inline bool chars_equal_insensitive(char a, char b) { return tolower(a) == tolower(b); }
inline bool chars_less_insensitive(char a, char b) { return tolower(a) < tolower(b); }