Fix behaviour of fence in dry runs

Also add a unit test and get rid of some utilities that are no longer necessary.
celerity · Aug 14, 2023 · ec07959 · ec07959
1 parent 97b900d
commit ec07959
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 52 deletions.
diff --git a/src/scheduler.cc b/src/scheduler.cc
@@ -19,7 +19,14 @@ namespace detail {
 
 	void abstract_scheduler::schedule() {
 		graph_serializer serializer([this](command_pkg&& pkg) {
-			if(m_is_dry_run && pkg.get_command_type() != command_type::epoch) { return; }
+			if(m_is_dry_run && pkg.get_command_type() != command_type::epoch && pkg.get_command_type() != command_type::fence) {
+				// in dry runs, skip everything except epochs and fences
+				return;
+			}
+			if(m_is_dry_run && pkg.get_command_type() == command_type::fence) {
+				CELERITY_WARN("Encountered a \"fence\" command while \"CELERITY_DRY_RUN_NODES\" is set. "
+				              "The result of this operation will not match the expected output of an actual run.");
+			}
 			// Executor may not be set during tests / benchmarks
 			if(m_exec != nullptr) { m_exec->enqueue(std::move(pkg)); }
 		});

diff --git a/test/runtime_tests.cc b/test/runtime_tests.cc
@@ -1188,9 +1188,10 @@ namespace detail {
 
 #endif
 
+	const std::string dryrun_envvar_name = "CELERITY_DRY_RUN_NODES";
+
 	void dry_run_with_nodes(const size_t num_nodes) {
-		const std::string dryrun_envvar_name = "CELERITY_DRY_RUN_NODES";
-		const auto ste = test_utils::set_test_env(dryrun_envvar_name, std::to_string(num_nodes));
+		env::scoped_test_environment ste(std::unordered_map<std::string, std::string>{{dryrun_envvar_name, std::to_string(num_nodes)}});
 
 		distr_queue q;
 
@@ -1222,6 +1223,27 @@ namespace detail {
 		dry_run_with_nodes(num_nodes);
 	}
 
+	TEST_CASE_METHOD(test_utils::runtime_fixture, "Dry run proceeds on fences", "[dryrun]") {
+		env::scoped_test_environment ste(std::unordered_map<std::string, std::string>{{dryrun_envvar_name, "1"}});
+
+		distr_queue q;
+
+		auto& rt = runtime::get_instance();
+		auto& tm = rt.get_task_manager();
+
+		REQUIRE(rt.is_dry_run());
+
+		buffer<bool, 0> buf{false};
+		q.submit([&](handler& cgh) {
+			accessor acc{buf, cgh, all{}, write_only_host_task};
+			cgh.host_task(on_master_node, [=] { acc = true; });
+		});
+
+		auto ret = experimental::fence(q, buf);
+		bool val = *ret.get(); // this will hang if fences are not processed in the dry run
+		CHECK_FALSE(val);      // extra check that the task was not actually executed
+	}
+
 	TEST_CASE_METHOD(test_utils::mpi_fixture, "Config reads environment variables correctly", "[env-vars][config]") {
 		std::unordered_map<std::string, std::string> valid_test_env_vars{
 		    {"CELERITY_LOG_LEVEL", "debug"},

diff --git a/test/test_utils.h b/test/test_utils.h
@@ -353,55 +353,6 @@ namespace test_utils {
 		}
 	}
 
-	class set_test_env {
-	  public:
-#ifdef _WIN32
-		set_test_env(const std::string& env, const std::string& val) : m_env_var_name(env) {
-			//  We use the ANSI version of Get/Set, because it does not require type conversion of char to wchar_t, and we can safely do this
-			//  because we are not mutating the text and therefore can treat them as raw bytes without having to worry about the text encoding.
-			const auto name_size = GetEnvironmentVariableA(env.c_str(), nullptr, 0);
-			if(name_size > 0) {
-				m_original_value.resize(name_size);
-				const auto res = GetEnvironmentVariableA(env.c_str(), m_original_value.data(), name_size);
-				assert(res != 0 && "Failed to get celerity environment variable");
-			}
-			const auto res = SetEnvironmentVariableA(env.c_str(), val.c_str());
-			assert(res != 0 && "Failed to set celerity environment variable");
-		}
-
-		~set_test_env() {
-			if(m_original_value.empty()) {
-				const auto res = SetEnvironmentVariableA(m_env_var_name.c_str(), NULL);
-				assert(res != 0 && "Failed to delete celerity environment variable");
-			} else {
-				const auto res = SetEnvironmentVariableA(m_env_var_name.c_str(), m_original_value.c_str());
-				assert(res != 0 && "Failed to reset celerity environment variable");
-			}
-		}
-
-#else
-		set_test_env(const std::string& env, const std::string& val) {
-			const char* has_value = std::getenv(env.c_str());
-			if(has_value != nullptr) { m_original_value = has_value; }
-			const auto res = setenv(env.c_str(), val.c_str(), 1);
-			assert(res == 0 && "Failed to set celerity environment variable");
-			m_env_var_name = env;
-		}
-		~set_test_env() {
-			if(m_original_value.empty()) {
-				const auto res = unsetenv(m_env_var_name.c_str());
-				assert(res == 0 && "Failed to unset celerity environment variable");
-			} else {
-				const auto res = setenv(m_env_var_name.c_str(), m_original_value.c_str(), 1);
-				assert(res == 0 && "Failed to reset celerity environment variable");
-			}
-		}
-#endif
-	  private:
-		std::string m_env_var_name;
-		std::string m_original_value;
-	};
-
 } // namespace test_utils
 } // namespace celerity