Skip to content

Optimize schema-comparator diffing performance#94

Merged
swalkinshaw merged 1 commit intomainfrom
schema-comparator-perf
Mar 23, 2026
Merged

Optimize schema-comparator diffing performance#94
swalkinshaw merged 1 commit intomainfrom
schema-comparator-perf

Conversation

@swalkinshaw
Copy link
Contributor

@swalkinshaw swalkinshaw commented Mar 17, 2026

Optimize schema-comparator diffing performance (~65% faster on large schemas)

Key optimizations:

  • Eliminate intermediate Vec allocations via diff_into(&mut Vec) pattern
  • Single-pass field matching: merge removals + common diffs into one iteration
  • Unified diff_directives_into replacing 3 separate directive functions
  • Ptr equality fast path for identical schemas (0µs)
  • HashMap fallback for types with 64+ fields (O(1) lookups on large types)
  • CriticalityLevel enum to avoid Cow string allocations during sorting
  • Early exits for empty directives/arguments, #[inline] hints on hot paths
  • Pre-allocate changes Vec with capacity 256
  • Criterion benchmark suite with real schema fixtures

Note: this is a manually curated (with the help of Claude) and cleaned up version of an /autoresearch run

@swalkinshaw swalkinshaw requested a review from adampetro March 17, 2026 16:21
@swalkinshaw swalkinshaw force-pushed the schema-comparator-perf branch from c4fb557 to 73d5d13 Compare March 17, 2026 16:45
Comment on lines +16 to +20
let src = std::fs::read_to_string(data_path(name)).unwrap();
let doc: DefinitionDocument = DefinitionDocument::parse(Box::leak(src.into_boxed_str()))
.result
.expect("Schema had parse errors");
let leaked = Box::leak(Box::new(doc));
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use include_str! to avoid some of the leaking? Or would that slow down compiles a lot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could impact compile time since the schema is massive, but anyway Claude said there's two leaks and this would only fix one since SchemaDefinition borrows from DefinitionDocument. Not sure it's a worth dealing with since it's only in the benchmark?

);
}

#[cold]
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL

Comment on lines -81 to -99
changes.extend(
directive_additions::<S, _>(self.old_type_definition, self.new_type_definition).map(
|directive| Change::DirectiveAdded {
directive,
location: DirectiveLocation::InputFieldDefinition,
member_name: self.old_field_definition.name(),
},
),
);

changes.extend(
directive_removals::<S, _>(self.old_type_definition, self.new_type_definition).map(
|directive| Change::DirectiveRemoved {
directive,
location: DirectiveLocation::InputFieldDefinition,
member_name: self.old_field_definition.name(),
},
),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this a bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep it was, the diff was checking the parent input object instead of the actual input field

@swalkinshaw swalkinshaw force-pushed the schema-comparator-perf branch from 73d5d13 to b352e79 Compare March 23, 2026 14:50
@swalkinshaw swalkinshaw requested a review from adampetro March 23, 2026 14:52
…schemas)

Key optimizations:
- Eliminate intermediate Vec allocations via diff_into(&mut Vec) pattern
- Single-pass field matching: merge removals + common diffs into one iteration
- Unified diff_directives_into replacing 3 separate directive functions
- Ptr equality fast path for identical schemas (0µs)
- HashMap fallback for types with 64+ fields (O(1) lookups on large types)
- CriticalityLevel enum to avoid Cow string allocations during sorting
- Early exits for empty directives/arguments, #[inline] hints on hot paths
- Pre-allocate changes Vec with capacity 256
- Criterion benchmark suite with real schema fixtures

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@swalkinshaw swalkinshaw force-pushed the schema-comparator-perf branch from b352e79 to 134c0c6 Compare March 23, 2026 14:53
@swalkinshaw swalkinshaw merged commit db55498 into main Mar 23, 2026
4 checks passed
@swalkinshaw swalkinshaw deleted the schema-comparator-perf branch March 23, 2026 16:52
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