Skip to content

Commit

Permalink
enable optimization in configure
Browse files Browse the repository at this point in the history
  • Loading branch information
@ulmus-scott @linuxdude42
ulmus-scott authored and linuxdude42 committed Nov 8, 2021
1 parent 5ac083c commit c123d2b
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 9 deletions.
15 changes: 10 additions & 5 deletions mythtv/configure
View file
Expand Up @@ -1996,8 +1996,6 @@ MYTHTV_CONFIG_LIST='
x11
libexiv2_external
libbluray_external
profiletype
debugtype
systemd_notify
systemd_journal
drm
Expand Down Expand Up @@ -2100,6 +2098,7 @@ MYTHTV_PATHS_LIST='
'

MYTHTV_CMDLINE_SET='
compile_type
logging
python
qmake
Expand Down Expand Up @@ -5144,16 +5143,20 @@ if test $target_os = android ; then
fi

append CCONFIG "$compile_type"
if test x$compile_type = x"profile" ; then
if test x$compile_type = x"release" ; then
add_cflags -DNDEBUG
add_cxxflags -DNDEBUG
check_cxxflags -O2
enable debug
elif test x$compile_type = x"profile" ; then
add_cflags -DNDEBUG
add_cxxflags -DNDEBUG
check_cxxflags -O2
enable debug
disable stripping
enable profiletype
elif test x$compile_type = x"debug"; then
enable debug
disable optimizations stripping
enable debugtype
fi

enabled silent_cc && append CCONFIG "silent"
Expand Down Expand Up @@ -7570,6 +7573,7 @@ QT_MIN_VERSION_HEX=$QT_MIN_VERSION_HEX
QT_MIN_VERSION=$QT_MIN_VERSION
MYTHTV_CONFIGURATION=$MYTHTV_CONFIGURATION
MYTHTV_CONFIG_MAK=1
COMPILE_TYPE=$compile_type
PREFIX=$prefix
prefix=$prefix
LIBDIRNAME=$libdir_name
Expand Down Expand Up @@ -7768,6 +7772,7 @@ cat > $TMPH <<EOF
#define QT_MIN_VERSION_STR "$QT_MIN_VERSION_STR"
#define QT_MIN_VERSION_HEX $QT_MIN_VERSION_HEX
#define QT_MIN_VERSION $QT_MIN_VERSION
#define COMPILE_TYPE "$compile_type"
#define av_restrict $_restrict
#define EXTERN_PREFIX "${extern_prefix}"
#define EXTERN_ASM ${extern_prefix}
Expand Down
4 changes: 2 additions & 2 deletions mythtv/libs/libmythbase/logging.cpp
View file
Expand Up @@ -405,7 +405,7 @@ bool LoggerThread::logConsole(LoggingItem *item) const
QString timestamp = item->getTimestampUs();
char shortname = item->getLevelChar();

#if CONFIG_DEBUGTYPE
#ifndef NDEBUG
if (item->tid())
{
line = qPrintable(QString("%1 %2 [%3/%4] %5 %6:%7:%8 %9\n")
Expand Down Expand Up @@ -466,7 +466,7 @@ bool LoggerThread::logConsole(LoggingItem *item) const
aprio = ANDROID_LOG_UNKNOWN;
break;
}
#if CONFIG_DEBUGTYPE
#ifndef NDEBUG
__android_log_print(aprio, "mfe", "%s:%d:%s %s", qPrintable(item->m_file),
item->m_line, qPrintable(item->m_function),
qPrintable(item->m_message));
Expand Down
4 changes: 2 additions & 2 deletions mythtv/programs/mythbackend/scheduler.cpp
View file
Expand Up @@ -611,9 +611,9 @@ void Scheduler::PrintRec(const RecordingInfo *p, const QString &prefix)
// is included. Because PrintList is 1 character longer than
// PrintRec, the output is off by 1 character. To compensate,
// initialize outstr to 1 space in those cases.
#if CONFIG_DEBUGTYPE
#ifndef NDEBUG // debug compile type
static QString initialOutstr = " ";
#else
#else // defined NDEBUG
static QString initialOutstr = "";
#endif

Expand Down

4 comments on commit c123d2b

@kmdewaal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be that this commit changes the default optimization from nothing to -O2 if you do just ./configure ? I did notice longer compilation times recently.

@ulmus-scott
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kmdewaal That is correct. However, ccache (or distcc) should keep the incremental compile times short.

Compiling with optimizations also enables some of the warning flags to actually work.

@kmdewaal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally do not approve of this commit because of the following reasons.

  • tracebacks are less informative due to information being optimized away
  • compilation time does increase.
  • yes I can do --compile-type=debug but then the lines in the log output become very long and less convenient to read
  • for my development way-of-working the previous default compile type of "profile" was just OK
  • If you want optimized code then you can use the --compile-type=release. Now the default compile type "profile" is almost equal to "release".
  • there is already extensive code checking in place with special builds to catch all kind of warnings. There is no need to change the default build for this.
    I think that it would have been better to propose and motivate this change in a git issue.

@ulmus-scott
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--extra-cxxflags=-Ox instead of --compile-type=debug where x is 0 for old behavior, i.e no optimizations at all, even ones you would expect, pessimized code to allow break points anywhere.

I'm looking into adding an easier optimization setting than above.

#406
#411 no discussion of optimization flags

I did mention this change on the mythtv-dev list.

Please sign in to comment.