Skip to content

Use model hash from metadata if available #25118

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.

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

Merged
merged 1 commit into from
Jun 20, 2025
Merged

Conversation

ashrit-ms
Copy link
Contributor

Description

This change implements Metadata-based hash override optimization to the model hashing logic. Added logic to check for "model_hash" in model metadata before computing hashes. If present, both model_graph_hash and model_weight_hash are set to the metadata value, bypassing computation entirely.

Motivation and Context

ONNX models generated using the Olive toolchain now have the option to include the model hash as part of the ONNX metadata. For such models, it would be beneficial to use the provided hash instead of computing it from scratch.

This change implements **Metadata-based hash override** optimization to
the model hashing logic. Added logic to check for "model_hash" in model
metadata before computing hashes. If present, both model_graph_hash and
model_weight_hash are set to the metadata value, bypassing computation
entirely.
@ashrit-ms ashrit-ms self-assigned this Jun 20, 2025
@ashrit-ms ashrit-ms requested review from Copilot and snnn June 20, 2025 02:22
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Implements a metadata-based override for model hashing to use a precomputed model_hash from ONNX metadata when available, avoiding costly recomputation.

  • Checks for a "model_hash" entry in model_->MetaData() and applies it to both graph and weight hashes
  • Falls back to ComputeModelGraphHash/ComputeModelWeightHash if no metadata key is present
  • Wrapping the override logic in a Windows-specific (_WIN32) conditional
Comments suppressed due to low confidence (3)

onnxruntime/core/session/inference_session.cc:2085

  • The metadata-based hash override is currently guarded by _WIN32. If this optimization should apply cross-platform, consider moving this logic outside the Windows-specific block.
      // Check if model metadata contains a "model_hash" field

onnxruntime/core/session/inference_session.cc:2089

  • [nitpick] Add or update documentation (in comments or external docs) to explain that metadata-based override bypasses hash computation and any implications for debugging.
      if (model_hash_it != metadata.end()) {

onnxruntime/core/session/inference_session.cc:2094

  • Add unit tests covering both branches (metadata present vs. absent) to ensure correct override behavior and fallback computation.
        // Compute hashes

@ashrit-ms ashrit-ms merged commit e0a4ed1 into main Jun 20, 2025
89 checks passed
@ashrit-ms ashrit-ms deleted the ashritms/skip-hash-olive branch June 20, 2025 03:06
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.

2 participants