From 84842d1f7764bb84006d945e2e244a0b26634877 Mon Sep 17 00:00:00 2001 From: Michel Van den Bergh Date: Mon, 13 Jan 2025 10:25:09 +0100 Subject: [PATCH] Include compiler and env in engine name. Retire clear_binaries. Currently the worker deletes cached binaries on restart. The reason is that the user may have changed compilers, so that new engine binaries would be matched against old engine binaries, built with a different compiler. In this PR we add compiler+version and a hash of selected environment variables to the engine name so that there is no risk of matching engine binaries built with different compilers, or with different environment variables. It makes little sense to hash PATH and TEMP but they should be included in the sandboxed environment used by the make command. A possible enhancement (not in this PR) would be to also add the full build command to the engine name so that engine binaries could even be preserved across worker updates (a worker update may change the build command). Raise worker version to 251 (also server side) --- server/fishtest/api.py | 2 +- worker/games.py | 43 ++++++++++++++++++++++++++++++++---------- worker/sri.txt | 2 +- worker/worker.py | 7 +------ 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/server/fishtest/api.py b/server/fishtest/api.py index f06a95061..49e8ec8f7 100644 --- a/server/fishtest/api.py +++ b/server/fishtest/api.py @@ -34,7 +34,7 @@ according to the route/URL mapping defined in `__init__.py`. """ -WORKER_VERSION = 250 +WORKER_VERSION = 251 @exception_view_config(HTTPException) diff --git a/worker/games.py b/worker/games.py index 35610b7f6..1bc43ec76 100644 --- a/worker/games.py +++ b/worker/games.py @@ -721,6 +721,7 @@ def setup_engine( concurrency, compiler, global_cache, + env, ): """Download and build sources in a temporary directory then move exe to destination""" tmp_dir = Path(tempfile.mkdtemp(dir=worker_dir)) @@ -768,10 +769,6 @@ def setup_engine( f"COMP={comp}", ] - # append -DNNUE_EMBEDDING_OFF to existing CXXFLAGS environment variable, if any - cxx = os.environ.get("CXXFLAGS", "") + " -DNNUE_EMBEDDING_OFF" - env = dict(os.environ, CXXFLAGS=cxx.strip()) - with subprocess.Popen( cmd, env=env, @@ -1208,7 +1205,6 @@ def run_games( run, task_id, pgn_file, - clear_binaries, global_cache, ): # This is the main fastchess driver. @@ -1298,7 +1294,7 @@ def parse_options(s): # Clean up old engines (keeping the num_bkps most recent). worker_dir = Path(__file__).resolve().parent testing_dir = worker_dir / "testing" - num_bkps = 0 if clear_binaries else 50 + num_bkps = 50 try: engines = sorted( testing_dir.glob("stockfish_*" + EXE_SUFFIX), @@ -1323,11 +1319,36 @@ def parse_options(s): sep="", file=sys.stderr, ) + + def create_environment(): + # OS and TEMP are necessary for msys2 + white_set = {"PATH", "OS", "TEMP"} + env = {k: v for k, v in os.environ.items() if k in white_set} + env["CXXFLAGS"] = "-DNNUE_EMBEDDING_OFF" + + # Do not hash directories such as PATH and TEMP + hash_set = {"OS", "CXXFLAGS"} + hashed_env = {k: v for k, v in env.items() if k in hash_set} + + env_hash = hashlib.sha256(str(hashed_env).encode()).hexdigest()[0:10] + return env, env_hash + + env, env_hash = create_environment() + + # build signature + bs = ( + worker_info["compiler"] + + "_" + + str(".".join([str(s) for s in worker_info["gcc_version"]])) + + "_env_" + + env_hash + ) # Create new engines. sha_new = run["args"]["resolved_new"] sha_base = run["args"]["resolved_base"] - new_engine_name = "stockfish_" + sha_new - base_engine_name = "stockfish_" + sha_base + + new_engine_name = "stockfish_" + sha_new + "_" + bs + base_engine_name = "stockfish_" + sha_base + "_" + bs new_engine = testing_dir / (new_engine_name + EXE_SUFFIX) base_engine = testing_dir / (base_engine_name + EXE_SUFFIX) @@ -1344,6 +1365,7 @@ def parse_options(s): worker_info["concurrency"], worker_info["compiler"], global_cache, + env, ) if not base_engine.exists(): setup_engine( @@ -1356,6 +1378,7 @@ def parse_options(s): worker_info["concurrency"], worker_info["compiler"], global_cache, + env, ) os.chdir(testing_dir) @@ -1565,7 +1588,7 @@ def make_player(arg): "-engine", "name=New-" + run["args"]["resolved_new"], "tc={}".format(scaled_new_tc), - "cmd=./{}".format(new_engine_name), + "cmd={}".format(new_engine), "dir=.", ] + new_options @@ -1574,7 +1597,7 @@ def make_player(arg): "-engine", "name=Base-" + run["args"]["resolved_base"], "tc={}".format(scaled_tc), - "cmd=./{}".format(base_engine_name), + "cmd={}".format(base_engine), "dir=.", ] + base_options diff --git a/worker/sri.txt b/worker/sri.txt index 9a7b95497..dcc663151 100644 --- a/worker/sri.txt +++ b/worker/sri.txt @@ -1 +1 @@ -{"__version": 250, "updater.py": "Mg+pWOgGA0gSo2TuXuuLCWLzwGwH91rsW1W3ixg3jYauHQpRMtNdGnCfuD1GqOhV", "worker.py": "rRWsXlFkv+whgfQS9IIJVEWnbquhlDa0DYRkkJ43/L7NTRR9pt2+sA2hqHH0x9M/", "games.py": "iYN33XDHqgfXnQheUtghu9GsuYhL0XU85/wUijSu87QtuwbyeAzJf2/Roqfatwgf"} +{"__version": 251, "updater.py": "Mg+pWOgGA0gSo2TuXuuLCWLzwGwH91rsW1W3ixg3jYauHQpRMtNdGnCfuD1GqOhV", "worker.py": "mfLxjM3Pq6448qSPwbLq5lTqX1nxMVBZbz8iv4Pkn9jPzhwHULsNCtFWw7ThcPAy", "games.py": "IZmJeczQV7IDMOvWS1NqR/tPe+Zg3rI8D1QDYIz/dvRE8auUU66PEjyfhWf9nbNe"} diff --git a/worker/worker.py b/worker/worker.py index 1cd10ddf9..4409f45ee 100644 --- a/worker/worker.py +++ b/worker/worker.py @@ -69,7 +69,7 @@ MIN_CLANG_MAJOR = 8 MIN_CLANG_MINOR = 0 -WORKER_VERSION = 250 +WORKER_VERSION = 251 FILE_LIST = ["updater.py", "worker.py", "games.py"] HTTP_TIMEOUT = 30.0 INITIAL_RETRY_TIME = 15.0 @@ -1298,7 +1298,6 @@ def fetch_and_handle_task( password, remote, current_state, - clear_binaries, global_cache, worker_lock, ): @@ -1391,7 +1390,6 @@ def fetch_and_handle_task( run, task_id, pgn_file, - clear_binaries, global_cache, ) success = True @@ -1609,14 +1607,12 @@ def worker(): # Start the main loop. delay = INITIAL_RETRY_TIME fish_exit = False - clear_binaries = True while current_state["alive"]: success = fetch_and_handle_task( worker_info, options.password, remote, current_state, - clear_binaries, options.global_cache, worker_lock, ) @@ -1637,7 +1633,6 @@ def worker(): safe_sleep(delay) delay = min(MAX_RETRY_TIME, delay * 2) else: - clear_binaries = False delay = INITIAL_RETRY_TIME if fish_exit: