Removed OpenMP-related code

After some discussion, we concluded that this code was unmaintained, not even used in
some places (display.cpp, units/frame.cpp), leaving the only area that really used it
at all the image surface cache. Considering there was never really a conclusive benchmark
of its benefits and because said surface cache will be used a lot less going forward,
we're just removing it and simplifying everything for everyone.

Closes wesnoth#1260 since it's now irrelevant.

(cherry-picked from commit 3792612)

CMakeLists.txt

@@ -56,7 +56,6 @@ option(ENABLE_SERVER "Enable compilation of MP server" ON)
 option(ENABLE_MYSQL "Enable building MP/add-ons servers with mysql support" OFF)
 option(ENABLE_TESTS "Build unit tests")
 option(ENABLE_NLS "Enable building of translations" ${ENABLE_GAME})
-option(ENABLE_OMP "Enables OpenMP, and has additional dependencies" OFF)
 option(ENABLE_HISTORY "Enable using GNU history for history in lua console" ON)
 
 # set what std version to use
@@ -447,12 +446,6 @@ if(DEFAULT_PREFS_FILE)
 	endif(NOT DEFAULT_PREFS_FILE MATCHES "^/")
 endif(DEFAULT_PREFS_FILE)
 
-if(ENABLE_OMP)
-	find_package(OpenMP REQUIRED)
-	set(CMAKE_C_FLAGS "${OpenMP_C_FLAGS} ${CMAKE_C_FLAGS}")
-	set(CMAKE_CXX_FLAGS "${OpenMP_CXX_FLAGS} ${CMAKE_C_FLAGS}")
-endif(ENABLE_OMP)
-
 if(ENABLE_DEBUG_WINDOW_LAYOUT)
 	add_definitions(-DDEBUG_WINDOW_LAYOUT_GRAPHS)
 endif(ENABLE_DEBUG_WINDOW_LAYOUT)

RELEASE_NOTES

@@ -69,6 +69,5 @@ KNOWN BUGS
 
 [list]
 [*]Trackpad tap clicking is sometimes not recognized ([url=http://forums.wesnoth.org/viewtopic.php?f=4&t=39788]forum post[/url]).
-[*]Unofficial builds with OpenMP support enabled randomly freeze (bug [issue]1260[/issue]).
 [/list]
 [/raissue]

SConstruct

@@ -108,7 +108,6 @@ opts.AddVariables(
     ('ctool', 'Set c compiler command if not using standard compiler.'),
     ('cxxtool', 'Set c++ compiler command if not using standard compiler.'),
     EnumVariable('cxx_std', 'Target c++ std version', '14', ['14', '17']),
-    BoolVariable('openmp', 'Enable openmp use.', False),
     ('sanitize', 'Enable clang and GCC sanitizer functionality. A comma separated list of sanitize suboptions must be passed as value.', ''),
     BoolVariable("fast", "Make scons faster at cost of less precise dependency tracking.", False),
     BoolVariable("lockfile", "Create a lockfile to prevent multiple instances of scons from being run at the same time on this working copy.", False),
@@ -471,9 +470,6 @@ for env in [test_env, client_env, env]:
             env.AppendUnique(CXXFLAGS = Split("-Wdocumentation -Wno-documentation-deprecated-sync"))
 
     if "gcc" in env["TOOLS"]:
-        if env['openmp']:
-            env.AppendUnique(CXXFLAGS = ["-fopenmp"], LIBS = ["gomp"])
-
         env.AppendUnique(CCFLAGS = Split("-Wno-unused-local-typedefs -Wno-maybe-uninitialized"))
         env.AppendUnique(CXXFLAGS = Split("-Wold-style-cast"))
 

changelog.md

@@ -43,6 +43,8 @@
  ### Multiplayer
    * A New Land: Fixed the scenario being broken.
    * Added team color to a few background units missing in Aethermaw.
+ ### Packaging
+   * OpenMP support has been removed. It is no longer an optional build-time dependency.
  ### User Interface
    * Swapped the position and formatting of game names and titles in the MP lobby.
    * Made Faction Select button's purpose more clear in MP Staging.

projectfiles/VC14/liblua.vcxproj

@@ -91,7 +91,8 @@
       <PreprocessorDefinitions>WIN32;_LIB;_CRT_SECURE_NO_WARNINGS;_SCL_SECURE_NO_WARNINGS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreadedDLL</RuntimeLibrary>
       <FunctionLevelLinking>true</FunctionLevelLinking>
-      <OpenMPSupport>true</OpenMPSupport>
+      <OpenMPSupport>
+      </OpenMPSupport>
       <WarningLevel>TurnOffAllWarnings</WarningLevel>
       <DebugInformationFormat>ProgramDatabase</DebugInformationFormat>
       <ForcedIncludeFiles>../src/wesnoth_lua_config.h;%(ForcedIncludeFiles)</ForcedIncludeFiles>

projectfiles/VC14/wesnoth.vcxproj

@@ -162,7 +162,8 @@
       <BufferSecurityCheck>false</BufferSecurityCheck>
       <FunctionLevelLinking>true</FunctionLevelLinking>
       <EnableEnhancedInstructionSet>StreamingSIMDExtensions2</EnableEnhancedInstructionSet>
-      <OpenMPSupport>true</OpenMPSupport>
+      <OpenMPSupport>
+      </OpenMPSupport>
       <WarningLevel>Level4</WarningLevel>
       <DebugInformationFormat>ProgramDatabase</DebugInformationFormat>
       <MultiProcessorCompilation>true</MultiProcessorCompilation>

projectfiles/VC14/wesnothd.vcxproj

@@ -115,7 +115,8 @@
       <PreprocessorDefinitions>WIN32;_WINSOCK_DEPRECATED_NO_WARNINGS;BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE;_WINDOWS;_CRT_SECURE_NO_WARNINGS;_WIN32_WINNT=_WIN32_WINNT_WINXP;HAVE_PYTHON;USE_GZIP;NOMINMAX;_SCL_SECURE_NO_WARNINGS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreadedDLL</RuntimeLibrary>
       <FunctionLevelLinking>true</FunctionLevelLinking>
-      <OpenMPSupport>true</OpenMPSupport>
+      <OpenMPSupport>
+      </OpenMPSupport>
       <WarningLevel>Level4</WarningLevel>
       <DebugInformationFormat>ProgramDatabase</DebugInformationFormat>
       <DisableSpecificWarnings>4503;4244;4127;</DisableSpecificWarnings>

projectfiles/VC14/wesnothlib.vcxproj

@@ -96,7 +96,8 @@
       <PreprocessorDefinitions>WIN32;_WINDOWS;_CRT_SECURE_NO_WARNINGS;HAVE_PYTHON;USE_GZIP;NO_HAVE_FRIBIDI;NOMINMAX;BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE;_SCL_SECURE_NO_WARNINGS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreadedDLL</RuntimeLibrary>
       <FunctionLevelLinking>true</FunctionLevelLinking>
-      <OpenMPSupport>true</OpenMPSupport>
+      <OpenMPSupport>
+      </OpenMPSupport>
       <WarningLevel>Level4</WarningLevel>
       <DebugInformationFormat>ProgramDatabase</DebugInformationFormat>
       <MultiProcessorCompilation>true</MultiProcessorCompilation>

src/build_info.cpp

@@ -257,11 +257,6 @@ version_table_manager::version_table_manager()
 	// Features table.
 	//
 
-	features.emplace_back(N_("feature^Experimental OpenMP support"));
-#ifdef _OPENMP
-	features.back().enabled = true;
-#endif
-
 	features.emplace_back(N_("feature^JPG screenshots"));
 #ifdef SDL_IMAGE_VERSION_ATLEAST
 #if SDL_IMAGE_VERSION_ATLEAST(2, 0, 2)

src/display.cpp

@@ -3088,56 +3088,23 @@ void display::invalidate_animations()
 		}
 	}
 
-#ifndef _OPENMP
-	for (const unit & u : dc_->units()) {
+	for(const unit& u : dc_->units()) {
 		u.anim_comp().refresh();
 	}
 	for (const unit* u : *fake_unit_man_) {
 		u->anim_comp().refresh();
 	}
-#else
-	std::vector<const unit *> open_mp_list;
-	for (const unit & u : dc_->units()) {
-		open_mp_list.push_back(&u);
-	}
-	// Note that it is an important assumption of the
-	// system that the fake units are added to the list
-	// after the real units, so that e.g. whiteboard
-	// planned moves are drawn over the real units.
-	for (const unit* u : *fake_unit_man_) {
-		open_mp_list.push_back(u);
-	}
-
-	// openMP can't iterate over std::size_t
-	const int omp_iterations = open_mp_list.size();
-	//#pragma omp parallel for shared(open_mp_list)
-	//this loop must not be parallelized. refresh is not thread-safe,
-	//for one, unit filters are not thread safe. this is because,
-	//adding new "scoped" wml variables is not thread safe. lua itself
-	//is not thread safe. when this loop was parallelized, assertion
-	//failures were reported in windows openmp builds.
-	for (int i = 0; i < omp_iterations; i++) {
-		open_mp_list[i]->anim_comp().refresh();
-	}
-#endif
 
 
 	bool new_inval;
 	do {
 		new_inval = false;
-#ifndef _OPENMP
 		for (const unit & u : dc_->units()) {
 			new_inval |=  u.anim_comp().invalidate(*this);
 		}
 		for (const unit* u : *fake_unit_man_) {
 			new_inval |=  u->anim_comp().invalidate(*this);
 		}
-#else
-	#pragma omp parallel for reduction(|:new_inval) shared(open_mp_list)
-		for (int i = 0; i < omp_iterations; i++) {
-				new_inval |= open_mp_list[i]->anim_comp().invalidate(*this);
-		}
-#endif
 	} while (new_inval);
 }
 

src/image.cpp

@@ -233,9 +233,6 @@ static int last_index_ = 0;
 
 void flush_cache()
 {
-#ifdef _OPENMP
-#pragma omp critical(image_cache)
-#endif //_OPENMP
 	{
 		images_.flush();
 		hexed_images_.flush();
@@ -1056,18 +1053,10 @@ surface get_image(const image::locator& i_locator, TYPE type)
 	}
 
 	// return the image if already cached
-	bool tmp;
-#ifdef _OPENMP
-#pragma omp critical(image_cache)
-#endif //_OPENMP
-	tmp = i_locator.in_cache(*imap);
+	bool tmp = i_locator.in_cache(*imap);
 
 	if(tmp) {
-		surface result;
-#ifdef _OPENMP
-#pragma omp critical(image_cache)
-#endif //_OPENMP
-		result = i_locator.locate_in_cache(*imap);
+		surface result = i_locator.locate_in_cache(*imap);
 		return result;
 	}
 
@@ -1096,9 +1085,6 @@ surface get_image(const image::locator& i_locator, TYPE type)
 		return res;
 	}
 
-#ifdef _OPENMP
-#pragma omp critical(image_cache)
-#endif //_OPENMP
 	i_locator.add_to_cache(*imap, res);
 
 	return res;
@@ -1165,9 +1151,6 @@ surface get_hexmask()
 bool is_in_hex(const locator& i_locator)
 {
 	bool result;
-#ifdef _OPENMP
-#pragma omp critical(in_hex_info_)
-#endif //_OPENMP
 	{
 		if(i_locator.in_cache(in_hex_info_)) {
 			result = i_locator.locate_in_cache(in_hex_info_);

src/units/frame.cpp

@@ -676,10 +676,6 @@ std::set<map_location> unit_frame::get_overlaped_hex(const int frame_time, const
 	} else {
 		int w = 0, h = 0;
 
-#ifdef _OPENMP
-#pragma omp critical(frame_surface) // with the way surfaces work it's hard to lock the refcount within sdl_utils
-#endif //_OPENMP
-
 		{
 			surface image;
 			if(!image_loc.is_void() && !image_loc.get_filename().empty()) { // invalid diag image, or not diagonal

src/wesnoth.cpp

@@ -116,10 +116,6 @@
 
 #include <windows.h>
 
-#if defined(_OPENMP) && _MSC_VER >= 1600
-#include <process.h>
-#endif
-
 #endif // _WIN32
 
 #ifdef DEBUG_WINDOW_LAYOUT_GRAPHS
@@ -960,42 +956,6 @@ static void wesnoth_terminate_handler(int)
 }
 #endif
 
-#if defined(_OPENMP) && _MSC_VER >= 1600
-static void restart_process()
-{
-	wchar_t process_path[MAX_PATH];
-	SetLastError(ERROR_SUCCESS);
-	GetModuleFileNameW(nullptr, process_path, MAX_PATH);
-
-	if(GetLastError() != ERROR_SUCCESS) {
-		throw std::runtime_error("Failed to retrieve the process path");
-	}
-
-	std::wstring commandline_str(GetCommandLineW());
-
-	// CreateProcessW is allowed to modify the passed command line.
-	// Therefore we need to copy it.
-	wchar_t* commandline_c_str = new wchar_t[commandline_str.length() + 1];
-	commandline_str.copy(commandline_c_str, commandline_str.length());
-	commandline_c_str[commandline_str.length()] = L'\0';
-
-	STARTUPINFOW startup_info;
-	ZeroMemory(&startup_info, sizeof(startup_info));
-	startup_info.cb = sizeof(startup_info);
-
-	PROCESS_INFORMATION process_info;
-	ZeroMemory(&process_info, sizeof(process_info));
-
-	CreateProcessW(
-		process_path, commandline_c_str, nullptr, nullptr, false, 0u, nullptr, nullptr, &startup_info, &process_info);
-
-	CloseHandle(process_info.hProcess);
-	CloseHandle(process_info.hThread);
-
-	std::exit(EXIT_SUCCESS);
-}
-#endif
-
 #ifdef _WIN32
 #define error_exit(res)                                                                                                \
 	do {                                                                                                               \
@@ -1057,32 +1017,6 @@ int main(int argc, char** argv)
 
 	assert(!args.empty());
 
-#ifdef _OPENMP
-	// Wesnoth is a special case for OMP
-	// OMP wait strategy is to have threads busy-loop for 100ms
-	// if there is nothing to do, they then go to sleep.
-	// this avoids the scheduler putting the thread to sleep when work
-	// is about to be available
-	//
-	// However Wesnoth has a lot of very small jobs that need to be done
-	// at each redraw => 50fps every 2ms.
-	// All the threads are thus busy-waiting all the time, hogging the CPU
-	// To avoid that problem, we need to set the OMP_WAIT_POLICY env var
-	// but that var is read by OMP at library loading time (before main)
-	// thus the relaunching of ourselves after setting the variable.
-#if !defined(_WIN32) && !defined(__APPLE__)
-	if(!getenv("OMP_WAIT_POLICY")) {
-		setenv("OMP_WAIT_POLICY", "PASSIVE", 1);
-		execv(argv[0], argv);
-	}
-#elif _MSC_VER >= 1600
-	if(!getenv("OMP_WAIT_POLICY")) {
-		_putenv_s("OMP_WAIT_POLICY", "PASSIVE");
-		restart_process();
-	}
-#endif
-#endif //_OPENMP
-
 	filesystem::init();
 
 	if(SDL_Init(SDL_INIT_TIMER) < 0) {