Skip to content

Stop re-hashing node source on every execution (OpenTelemetry, caching) - #1756

Open
Thanatos9404 wants to merge 1 commit into
apache:mainfrom
Thanatos9404:cache-node-version-hash
Open

Thanatos9404 wants to merge 1 commit into
apache:mainfrom
Thanatos9404:cache-node-version-hash

Conversation

@Thanatos9404

Copy link
Copy Markdown

Fixes #1714

Graph execution hooks get a new HamiltonGraph from HamiltonGraph.from_graph on every run, so the cached_property versions never carry over to the next run. OpenTelemetryTracer reads graph.version before every execution, so it hashed the source of every node every time. .with_cache() pays the same cost: resolve_code_versions reads HamiltonNode.from_node(node).version for every node on the execution path.

Changes

HamiltonNode.version now goes through a private helper, _hash_function_source, which keeps hash_source_code(fn, strip=True) results in a module-level weakref.WeakKeyDictionary keyed by the originating function.

  • Each entry stores the code object of inspect.unwrap(fn) (the function inspect.getsource reads) and is recomputed when it changes. IPython's autoreload, which the Jupyter how-to recommends, swaps __code__ on the existing function; without this check .with_cache() could return results from the old code.
  • Entries go away with their function. Callables without a code object, or that cannot be hashed or weakly referenced, are hashed every time as before. Errors are not cached.
  • Version values are unchanged and hash_source_code is untouched.

Alternatives:

  • lru_cache on hash_source_code would return stale hashes after autoreload, also to h_diskcache and CacheAdapter, and keeps every function alive.
  • Storing the version on node.Node, or reusing one HamiltonGraph per FunctionGraph, would also go stale under autoreload, since those objects outlive the reload, so they would need the same check. Keying by function also hashes a function shared by several nodes (@parameterize, or an input and its declaring function) only once.

How I tested this

  • tests/test_graph_types.py: one hash across three HamiltonNode wrappers; the version follows a replaced __code__ and a replaced __wrapped__.
  • tests/plugins/test_h_opentelemetry.py (new): the second traced execution hashes nothing, the first hashes each function once, and node span versions equal the direct hashes.

On main the two counting tests fail (assert 3 == 1, assert 8 == 4) and the other three pass. On this branch they pass on Python 3.10, 3.12 and 3.14, as do tests/lifecycle and tests/caching; prek run passes on the changed files.

Hash calls per execution, chain of 500 functions plus one input, 10 runs:

Driver main This branch
no adapters 0 0
OpenTelemetryTracer 501 every run 500 on run 1, then 0
.with_cache() 500 every run 500 on run 1, then 0

With the tracer, runs 2 to 10 took 76 to 130 ms on main and 9 to 21 ms here (2 to 5 ms without adapters). The first run costs about the same; the graph version is identical.

Notes

  • If a file is edited without a reload, a function already hashed in this process keeps the hash of the running code, where main would hash the new file text. as_dict()["source"] still reads from disk, so the two can then disagree.
  • The OpenTelemetry tests only run where opentelemetry-sdk is installed: in CI it comes through mlflow on Python 3.13 and below, and the file is skipped on 3.14. I can add opentelemetry-sdk to the test group if you prefer.

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality. (No user-facing change.)

Graph execution hooks receive a new HamiltonGraph from
HamiltonGraph.from_graph on every run, so the cached `version` properties
never survive to the next execution. Reading `graph.version` in the
OpenTelemetry tracer, or resolving code versions in the caching adapter,
therefore hashed the source of every node on every execution.

HamiltonNode.version now reuses the hash from a module-level
WeakKeyDictionary keyed by the originating function. Each entry records the
code object it was computed from (following `__wrapped__`, as
inspect.getsource does), so a function whose code is replaced in place, for
example by IPython's autoreload, is hashed again. The version values are
unchanged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenTelemetry recomputes graph and node source hashes on every execution

1 participant