Skip to content

[Benchmarks] Print all important env vars in debug logs #20415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: sycl
Choose a base branch
from
+14 −8
Open

[Benchmarks] Print all important env vars in debug logs #20415

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c171cb
[Benchmarks] Print all important env vars in debug logs
PatKamin Oct 21, 2025
d24f622
Print only added paths to PATH and LD_LIBRARY_PATH
PatKamin Oct 22, 2025
aa3f3ea
Replace env vars where appropriate
PatKamin Oct 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions devops/scripts/benchmarks/utils/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,31 +49,37 @@ def run(
command = command.split()

env = os.environ.copy()

for ldlib in ld_library:
if os.path.isdir(ldlib):
env["LD_LIBRARY_PATH"] = (
ldlib + os.pathsep + env.get("LD_LIBRARY_PATH", "")
env_vars["LD_LIBRARY_PATH"] = os.pathsep.join(
filter(None, [ldlib, env_vars.get("LD_LIBRARY_PATH", "")])
)
else:
log.warning(f"LD_LIBRARY_PATH component does not exist: {ldlib}")

# order is important, we want provided sycl rt libraries to be first
if add_sycl:
sycl_bin_path = os.path.join(options.sycl, "bin")
env["PATH"] = sycl_bin_path + os.pathsep + env.get("PATH", "")
env_vars["PATH"] = os.pathsep.join(
filter(None, [sycl_bin_path, env_vars.get("PATH", "")])
)
sycl_lib_path = os.path.join(options.sycl, "lib")
env["LD_LIBRARY_PATH"] = (
sycl_lib_path + os.pathsep + env.get("LD_LIBRARY_PATH", "")
env_vars["LD_LIBRARY_PATH"] = os.pathsep.join(
filter(None, [sycl_lib_path, env_vars.get("LD_LIBRARY_PATH", "")])
)

env.update(env_vars)

command_str = " ".join(command)
env_str = " ".join(f"{key}={value}" for key, value in env_vars.items())
full_command_str = f"{env_str} {command_str}".strip()
log.debug(f"Running: {full_command_str}")

for key, value in env_vars.items():
Copy link
Contributor

Choose a reason for hiding this comment

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

now, reading it for the second time I'm wondering.. why do we even have this env_vars? just to print it? If so, why not skip env_vars, set env above, and just print in debug log PATH and LD_LIBRARY_PATH from env...?

Right now, we print something (env_vars), but actually using something else (env).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to clutter verbose logs with paths in PATH that are not relevant to benchmarks. It's better to have only relevant paths co I could reproduce easily the env needed to run given commands by hand on the same or other machine.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes and no, the other PATHs can actually influence the benchmark... ;)

I still believe my point is valid - we print something (env_vars), but actually using something else for running the command (env).

Anyway, I guess we can live with the currently proposed implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I know that the other PATHs could make a difference, but see what is printed now... no PATHs :) From my experience, only these added here are relevant for a quick debug. And that's what I was missing - we printed some env vars, but they were not enough to actually use the printed command and execute it apart from the framework.

# Only PATH and LD_LIBRARY_PATH should be prepended to existing values
if key in ("PATH", "LD_LIBRARY_PATH") and (old := env.get(key)):
env[key] = os.pathsep.join([value, old])
else:
env[key] = value

# Normalize input to bytes if it's a str
if isinstance(input, str):
input_bytes = input.encode()
Expand Down
Loading