effect icon indicating copy to clipboard operation
effect copied to clipboard

`Schema.hash` / `AST.hash` should not use JSON.stringify

Open schickling opened this issue 1 year ago • 1 comments

What is the problem this feature would solve?

Currently Schema.hash / AST.hash is implemented using Hash.string(JSON.stringify(ast, null, 2)) which is inefficient and for larger schemas turns into a bottleneck or can fail all together with RangeError: Invalid string length

CleanShot 2024-05-09 at 16 24 09@2x

What is the feature you are proposing to solve the problem?

Ideally the hashing mechanism was implemented on a case by case basis on the AST level itself instead of relying on stringifying the AST.

Improving this implementation could also help to make hashes more stable across different Effect schema versions.

What alternatives have you considered?

No response

schickling avatar May 09 '24 14:05 schickling

Hashing should use the same strategy we use to hash structs but we should probably add, in addition to the plain hash, something like a crc32. Reason being that hash isn't supposed to give hints about equality it is only supposed to bucket and optimise equality checks. If we instead need a high-probability indication of difference we need a checksum.

I can do some research on crc when I get back, for plain hash this should quite literally follow the same code we have for data classes / etc

mikearnaldi avatar May 09 '24 14:05 mikearnaldi