feat(pathfinder): opt-in diagnostic logging for load_nvidia_dynamic_lib - #2513
feat(pathfinder): opt-in diagnostic logging for load_nvidia_dynamic_lib#2513u7k4rs6 wants to merge 1 commit into
Conversation
Adds a `cuda.pathfinder` logger, disabled by default, enabled by setting CUDA_PATHFINDER_LOG_LEVEL to a standard level name or an integer. `logging` is imported only when the variable is set. Importing it pulls in seven additional modules and measurably slows `import cuda.pathfinder`, which sits on the import hot path of every consumer, so the disabled path imports nothing and costs one module-global lookup plus an identity check (~5 ns) at each call site. No message string or `extra` dict is built when logging is off. Instruments the dynamic-library search only: each find step reports whether it matched, a successful load reports the resolved path and `found_via`, and the failure path emits the accumulated candidate list as structured fields. Records carry `pathfinder_*` fields so consumers can filter without parsing messages. The logger attaches a NullHandler, never calls basicConfig, and never touches the root logger. Invalid CUDA_PATHFINDER_LOG_LEVEL values warn once and leave logging disabled rather than raising. The environment variable is read once at import, matching the documented read-once policy of get_cuda_path_or_home(). Signed-off-by: Utkarsh Bahuguna <utkarshbahuguna10@gmail.com>
github-actions
Bot
added
the
cuda.pathfinder
|
@rwgk Please review this. |
|
@rparolin Thanks. Also flagging that copy-pr-bot is still holding workflows pending vetting, so nothing has run on this yet. Happy to un-draft once CI can report. |
|
/ok to test |
@rparolin, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/ |
|
@rparolin The /ok to test came back with E1. That doc page is behind SSO so I can't read it, but the PR was still a draft when you ran it and it's out of draft now, so a retry may go through: /ok to test fbfe6de. Also, un-drafting made pr-metadata-check run for real and it's failing on the type label. I have cuda.pathfinder but none of feature/enhancement/etc., and I can't set labels. feature would match #650. |
2 participants
Description
Implements the interface proposed in #650. Opening as a draft: the direction was
proposed on 4 Jun and pinged since without a reply, so this is a concrete shape to
react to rather than a request to merge. Naming (logger name, env var) and level
choices are open to whatever the maintainers prefer.
Scoped to
load_nvidia_dynamic_libonly. The same pattern extends to the otherfour finder families --
find_nvidia_binary_utility,find_nvidia_header_directory,find_static_lib,find_bitcode_lib-- and I'll extend it if you like the shape.What
cuda.pathfinderlogger, disabled by default, enabled viaCUDA_PATHFINDER_LOG_LEVEL(level name or integer).path and
found_via; the failure path logs the accumulated candidate list.pathfinder_*fields (pathfinder_libname,pathfinder_found_via,pathfinder_abs_path,pathfinder_step,pathfinder_matched, ...) so consumerscan filter structurally instead of regexing messages.
found_viaalready existedon
FindResultandLoadedDL; this surfaces it rather than inventing a field.basicConfig, no root-logger configuration.On off-by-default
@rwgk argued against opt-in diagnostics in #1034, and that concern is worth
answering directly rather than leaving implicit.
SearchContext.raise_not_found()already formats the full candidate list intoDynamicLibNotFoundError, always on. The case being protected there -- someonehits a failure and then needs a second run with a flag to diagnose it -- is
already covered today, and this PR does not change it. What is off by default is
success-path telemetry, where by definition nothing needs diagnosing. Because
loggingis imported lazily, the disabled path costs nothing at all, sooff-by-default is not a tradeoff being made against visibility.
There is one real gap, and I don't want to gloss it: a load that succeeds but
resolves the wrong library.
found_viabeing off by default does cost somethingthere.
I looked at whether
warnings.warn-- already the house idiom, perenv_vars.py:96andload_dl_windows.py:90-- would fit a genuinely suspicioussuccess better than a log line. I don't think it does. Both existing uses signal
an actionable anomaly, and neither shape applies:
that a conda copy shadowed a site-packages copy would mean continuing the search
after a hit -- real filesystem work on every successful load.
was_already_loaded_from_elsewhere. Warning on it would be wrong: it is thenormal case whenever two NVIDIA libraries share a dependency, so it would fire
constantly on correct usage.
So it stays a log field. If you'd rather have an always-on warning for some
narrower condition, I'm happy to add it -- I just couldn't find one that is both
cheap to detect and reliably suspicious.
Import cost
import loggingpulls in seven modules (logging,atexit,string,_string,textwrap,traceback,_colorize). Measured on this branch:sys.modulesafterimport cuda.pathfinderFalseTruePer call site, disabled: 29.7 ns vs 24.7 ns baseline over 500k iterations.
Provenance
Written with AI assistance. The new tests carry
@pytest.mark.agent_authored(model="claude-opus-5"), the convention documentedin
AGENTS.md; flagging it here too since that marker only covers tests, andthis PR also adds source under
cuda/pathfinder/_utils/.Checklist
Docs box unticked deliberately: nothing user-facing is documented yet, and where
it belongs (
docs/source/, the env-var list, or underload_nvidia_dynamic_lib)is worth a steer first.