Pass precomputed hash to SerializedDagModel constructor - #71222
Open
ahujaanmol1289 wants to merge 1 commit into
Open
Pass precomputed hash to SerializedDagModel constructor#71222ahujaanmol1289 wants to merge 1 commit into
ahujaanmol1289 wants to merge 1 commit into
Conversation
…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
force-pushed
the
fix/pass-precomputed-hash-clean
branch
from
August 6, 2026 09:23
25e578d to
7efd83e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
write_dag()computescls.hash(dag.data)to compare against the storedhash. When the DAG has changed, it then calls
cls(dag)which recomputesthe same hash internally. This PR passes the already-known hash via
_precomputed_hashto skip the redundant computation.When
reused_deadline_datamutatesdag.dataafter the initial hashcomparison, 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 onthe changed-DAG path without touching the unchanged-DAG path or altering
storage format.
What does NOT change
hash()remains a public classmethod with the same signaturetest_serialized_dagtests pass unmodifiedTesting
5 new tests in
TestPrecomputedHash:_precomputed_hashskips internalhash()callhash()called normally (backward compat)write_dagpasses precomputed hash on changed-DAG path