Rework Bytes Chunk type and tweak comparison#6112
Conversation
`Chunk` is now a wrapped `ByteArray` similar to vector, but it takes advantage of some low level possibilities instead of implementing various things via stream fusion. The latter seems to be pretty poor in some cases, like comparison. Comparison of `Rope` optimistically looks for pairs of `One` values to try a faster path. Avoid parameterizing `universalEq/Compare` by a comparison function. We're never going to dynamically expand the set of equalities at runtime, which was the idea behind that, and it likely just costs performance.
|
I've confirmed that the Unison Cloud integration tests pass with these changes. I know that benchmarks can be deceiving, but since these changes were (presumably) made in the name of performance, are there any benchmark results showing the impact? |
|
@ceedubs Sure. I was running benchmarks like these That compares a pair of UUID-sized bytes. I also tried what would happen if you used This doesn't measure the absolute performance of the operations, because the loop itself is non-trivial overhead. But I thought that was more representative of a scenario they'd actually be used in (like map insertion). Anyhow...
|
|
Awesome. Thanks @dolio! |
|
I also have some benchmarks showing ~ 3-5x speed improvement for Map insertion and lookup when using Bytes keys. |
Chunkis now a wrappedByteArraysimilar to vector, but it takes advantage of some low level possibilities instead of implementing various things via stream fusion. The latter seems to be pretty poor in some cases, like comparison.Comparison of
Ropeoptimistically looks for pairs ofOnevalues to try a faster path.Avoid parameterizing
universalEq/Compareby a comparison function. We're never going to dynamically expand the set of equalities at runtime, which was the idea behind that, and it likely just costs performance.I took the opportunity to implement
encodeNat...using the endian tests and a bulk array write. I wanted to do this back when I was fiddling with the bytes read replacements, but it wasn't as convenient when usingVector Word8.basetests and transcripts pass. If @ceedubs could do some cloud testing that'd be appreciated, since I think they probably make more thorough use of this stuff.