search

apollographql / apollo-rs

Spec compliant GraphQL Tools in Rust.
Apache License 2.0
566 stars 43 forks source link

Remove `Hash` cache in `Node<T>` #872

Closed SimonSapin closed 2 months ago

SimonSapin commented 2 months ago

Hashing is much less important to apollo-compiler as it was when we were using Salsa. Many of the structs stored in Node<_> don’t even implement Hash since they contain IndexMap which doesn’t.

We lose: potential CPU time if repeatedly hashing nodes. ast::Definition implements PartialEq and Hash by traversing the whole subtree, which can be expensive. We gain: 8 fewer bytes in the heap allocation of every Node, and slightly less code complexity.

SimonSapin commented 2 months ago

Added a changelog entry, and the Borrow impl that would have been semantically incorrect with the cache.