Skip to content

Commit

Permalink
Removed OpenMP code (closes #4130)
Browse files Browse the repository at this point in the history 
We already concluded a long time ago the introduction of OMP was quite ineffective at speeding up
performance
  • Loading branch information
@Vultraz
Vultraz committed Jul 25, 2019
1 parent dade3d0 commit 74bf066
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 136 deletions.
5 changes: 0 additions & 5 deletions src/build_info.cpp
View file
Expand Up @@ -265,11 +265,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)
Expand Down
46 changes: 0 additions & 46 deletions src/display.cpp
View file
Expand Up @@ -2979,9 +2979,6 @@ void display::invalidate_all()
{
DBG_DP << "invalidate_all()\n";
invalidateAll_ = true;
#ifdef _OPENMP
#pragma omp critical(invalidated_)
#endif //_OPENMP
invalidated_.clear();
}

Expand All @@ -2991,9 +2988,6 @@ bool display::invalidate(const map_location& loc)
return false;

bool tmp;
#ifdef _OPENMP
#pragma omp critical(invalidated_)
#endif //_OPENMP
tmp = invalidated_.insert(loc).second;
return tmp;
}
Expand All @@ -3004,9 +2998,6 @@ bool display::invalidate(const std::set<map_location>& locs)
return false;
bool ret = false;
for (const map_location& loc : locs) {
#ifdef _OPENMP
#pragma omp critical(invalidated_)
#endif //_OPENMP
ret = invalidated_.insert(loc).second || ret;
}
return ret;
Expand All @@ -3021,9 +3012,6 @@ bool display::propagate_invalidation(const std::set<map_location>& locs)
return false; // propagation never needed

bool result = false;
#ifdef _OPENMP
#pragma omp critical(invalidated_)
#endif //_OPENMP
{
// search the first hex invalidated (if any)
std::set<map_location>::const_iterator i = locs.begin();
Expand Down Expand Up @@ -3085,56 +3073,22 @@ void display::invalidate_animations()
}
}

#ifndef _OPENMP
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 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);
}

Expand Down
15 changes: 0 additions & 15 deletions src/picture.cpp
View file
Expand Up @@ -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();
Expand Down Expand Up @@ -1004,16 +1001,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);

if(tmp) {
surface result;
#ifdef _OPENMP
#pragma omp critical(image_cache)
#endif //_OPENMP
result = i_locator.locate_in_cache(*imap);
return result;
}
Expand Down Expand Up @@ -1043,9 +1034,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;
Expand Down Expand Up @@ -1112,9 +1100,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_);
Expand Down
4 changes: 0 additions & 4 deletions src/units/frame.cpp
View file
Expand Up @@ -682,10 +682,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
Expand Down
66 changes: 0 additions & 66 deletions src/wesnoth.cpp
View file
Expand Up @@ -116,10 +116,6 @@

#include <windows.h>

#if defined(_OPENMP) && _MSC_VER >= 1600
#include <process.h>
#endif

#endif // _WIN32

#ifdef DEBUG_WINDOW_LAYOUT_GRAPHS
Expand Down Expand Up @@ -958,42 +954,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 { \
Expand Down Expand Up @@ -1054,32 +1014,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) {
Expand Down

0 comments on commit 74bf066

Please sign in to comment.