Skip to content

Avoid ParseTensorData's double copy in CSE tensor hash/equality for raw_data - #8

Open
take-cheeze wants to merge 1 commit into
claude/graph-native-optimize-633from
claude/eliminate-duplicate-initializer-perf-633
Open

Avoid ParseTensorData's double copy in CSE tensor hash/equality for raw_data#8
take-cheeze wants to merge 1 commit into
claude/graph-native-optimize-633from
claude/eliminate-duplicate-initializer-perf-633

Conversation

@take-cheeze

@take-cheeze take-cheeze commented Aug 18, 2026

Copy link
Copy Markdown
Member

Summary

CSETensorHash and CSETensorEqual (shared by eliminate_duplicate_initializer and eliminate_common_subexpression) went through ParseTensorData<T> for every tensor comparison. For raw_data-backed tensors that makes two full copies of the tensor's bytes per call — one to un-const the std::string, one to convert it into a typed std::vector — plus an element-by-element hash_combine loop over every element.

Since ONNX's raw_data is always little-endian on disk regardless of host byte order, byte-identical raw_data always implies value-identical data, on any host. This adds a fast path that hashes/compares the raw bytes directly via Tensor::raw() (a zero-copy const std::string&) whenever both tensors are raw_data-backed, skipping ParseTensorData entirely.

Base branch note: this targets claude/graph-native-optimize-633 (the commit onnxsim's master actually has third_party/onnx-optimizer pinned to) rather than main, since main is missing some already-adopted changes (e.g. InitializersAsConstants) that this fix needs to build against for a clean diff. Retarget to main once that catches up.

Why this can't be a false positive

  • CSETensorEqual: byte-identical raw_data ⟹ value-identical (decoding bytes is a pure, deterministic function of those bytes), so lhs->raw() == rhs->raw() can never merge two tensors with actually-different values.
  • CSETensorHash: the only requirement on a hash function is that equal tensors hash equally. Since the fast-path equality check above compares the exact same raw() bytes, hashing those same bytes preserves that invariant.
  • A tensor whose duplicate happens to be stored via typed fields instead of raw_data (rare in practice — real exporters use one storage format consistently) simply won't hash/compare equal through this fast path, so it may go undetected as a duplicate. That's a missed optimization, never an incorrect merge — consistent with this pass's existing doc comment that it's a best-effort Nop-type optimization.

Motivation

Found while investigating onnxsim#633 (onnxsim vs onnxslim speed gap). Profiling mixer_l16_224_in21k (582 nodes, ~300MB of raw_data initializers, 53 fixed-point rounds) showed eliminate_duplicate_initializer alone accounted for ~98% of all optimizer pass time (up to 302s of 306s), because it re-hashes every initializer from scratch on every round even though most initializer content is unchanged round to round. With this fix, total onnxsim.simplify() time on that model roughly halves (268s → 111s in one measurement).

Testing

  • Added/verified correctness with a hand-built model containing two byte-identical raw-data initializers used by different nodes — eliminate_duplicate_initializer still merges them to one, and the resulting model produces bit-identical inference output (verified via onnxruntime).
  • Confirmed against onnxsim's existing test_eliminate_duplicate_initializer coverage (INT32/INT64/FLOAT/DOUBLE, all raw=True) — all pass.
  • Ran onnxsim's broader Python test suite (test_simple.py, test_backend.py, test_model_checking.py, test_constant_fold_determinism.py, test_moved_optimizer_passes.py): 51 passed, 2 failed on an unrelated missing optional dependency (onnxscript), 0 regressions.
  • Re-measured mixer_l16_224_in21k end-to-end before/after on both onnxsim's default (ModelProto) and opt-in graph-native optimize paths — both show ~2.4x-4x speedups, with identical output node counts (no change in simplification result).

…aw_data

CSETensorHash and CSETensorEqual (used by eliminate_duplicate_initializer
and eliminate_common_subexpression) went through ParseTensorData<T> for
every tensor comparison, which for raw_data-backed tensors makes two full
copies of the tensor's bytes (one to un-const the string, one to convert
it into a typed std::vector) plus an element-by-element hash_combine loop.

Since raw_data is always little-endian on disk regardless of host byte
order, byte-identical raw_data always implies value-identical data on any
host. Add a fast path that hashes/compares the raw bytes directly via
Tensor::raw() (a zero-copy const std::string&) when both tensors are
raw_data-backed, skipping ParseTensorData entirely. A tensor whose
duplicate happens to be stored via typed fields instead of raw_data
(rare in practice) simply won't be recognized as a duplicate through
this path -- a missed optimization, never an incorrect merge.

On a 582-node ONNX model with ~300MB of raw_data initializers,
eliminate_duplicate_initializer accounted for ~98% of all optimizer pass
time (re-hashing every initializer from scratch on each of ~53
fixed-point rounds); this fix cuts total simplify() time roughly in
half on that model.
@take-cheeze
take-cheeze force-pushed the claude/eliminate-duplicate-initializer-perf-633 branch from 6604bc2 to 229d9bc Compare August 18, 2026 07:40
@take-cheeze
take-cheeze changed the base branch from main to claude/graph-native-optimize-633 August 18, 2026 07:40
pull Bot pushed a commit to naonao-cola/onnx-simplifier that referenced this pull request Aug 18, 2026
Points at onnxsim/optimizer#8, which avoids ParseTensorData's double
copy in CSETensorHash/CSETensorEqual for raw_data-backed tensors.
eliminate_duplicate_initializer re-hashes every initializer's full byte
content from scratch on every fixed-point round; on mixer_l16_224_in21k
(~300MB of raw_data initializers, 53 rounds) that pass alone accounted
for ~98% of all optimizer pass time. This fix roughly halves total
simplify() time on that model, on both the default and opt-in
graph-native optimize paths.
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