Skip to content

Pass precomputed hash to SerializedDagModel constructor - #71222

Open
ahujaanmol1289 wants to merge 1 commit into
apache:mainfrom
ahujaanmol1289:fix/pass-precomputed-hash-clean
Open

Pass precomputed hash to SerializedDagModel constructor#71222
ahujaanmol1289 wants to merge 1 commit into
apache:mainfrom
ahujaanmol1289:fix/pass-precomputed-hash-clean

Conversation

@ahujaanmol1289

@ahujaanmol1289 ahujaanmol1289 commented Aug 6, 2026

Copy link
Copy Markdown

What this does

write_dag() computes cls.hash(dag.data) to compare against the stored
hash. When the DAG has changed, it then calls cls(dag) which recomputes
the same hash internally. This PR passes the already-known hash via
_precomputed_hash to skip the redundant computation.

When reused_deadline_data mutates dag.data after the initial hash
comparison, the precomputed hash is stale, so it is recomputed once more
right after the mutation before being passed in.

Why

Addresses the feedback from #70715 — the reviewer correctly pointed out
that the original approach (merging hash and storage JSON generation)
regressed the common unchanged-DAG path and altered stored bytes. This
approach is minimal: it only eliminates the redundant hash() call on
the changed-DAG path without touching the unchanged-DAG path or altering
storage format.

What does NOT change

  • The hash value produced is identical to before
  • The stored JSON is identical
  • hash() remains a public classmethod with the same signature
  • The unchanged-DAG (common) path is untouched — no performance regression
  • All existing test_serialized_dag tests pass unmodified

Testing

5 new tests in TestPrecomputedHash:

  • _precomputed_hash skips internal hash() call
  • Storage bytes identical with and without precomputed hash
  • Without precomputed hash, hash() called normally (backward compat)
  • write_dag passes precomputed hash on changed-DAG path
  • Deadline UUID mutation does not pass stale hash

…omputes cls.hash(dag.data) to compare against the stored hash. When the DAG has changed, it then calls cls(dag) which recomputes the same hash internally. Pass the already-known hash via _precomputed_hash to skip the redundant computation.

When reused_deadline_data mutates dag.data after the initial hash, the precomputed hash is stale, so the hash is recomputed once more right after the mutation before being passed in.
Addresses the feedback from apache#70715.
@ahujaanmol1289
ahujaanmol1289 force-pushed the fix/pass-precomputed-hash-clean branch from 25e578d to 7efd83e Compare August 6, 2026 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants