Use ShortByteString for OneEraHash and ConvertRawHash #2266
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mrBliss
force-pushed
the
mrBliss/short-bytestring
branch
from
86748c6 to
f5950f3
Compare
mrBliss
commented
Jun 15, 2020
There was a problem hiding this comment.
Currently a draft, as we currently require going through a regular ByteString for both Byron and Shelley (see my inline comments).
If/when we decide to switch to ShortByteStrings, it would be good to measure the actual difference in memory usage.
UPDATE: both Byron's and Shelley's hash type now uses a ShortByteString under the hood and expose the necessary conversions (avoiding going through an intermediary Strict.ByteString).
ouroboros-consensus-byron/src/Ouroboros/Consensus/Byron/Ledger/Block.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/Block.hs
Outdated
Show resolved
Hide resolved
mrBliss
force-pushed
the
mrBliss/short-bytestring
branch
from
f5950f3 to
12820e3
Compare
Merged
mrBliss
force-pushed
the
mrBliss/short-bytestring
branch
from
12820e3 to
dea8236
Compare
mrBliss
changed the title
Since the Byron and Shelley hash types are `ShortByteString`s under the hood, switch over the representation of `OneEraHash` from `Strict.ByteString` to `ShortByteString` too. To avoid the intermediary `Strict.ByteString` in the conversion, add `toShortRawHash` and `fromShortRawHash` to `ConvertRawHash`. This avoids the memory overhead that a regular strict `ByteString` has. According to the [Haddocks of `ShortByteString`][SBS], using a `ByteString` (unshared) for a 32 byte hash (which is the case for Byron and Shelley) requires 72 + 32 bytes, but a `ShortByteString` requires only 32 + 32 bytes. This can make a noticeable difference in memory usage as we're keeping lots of hashes in memory: * O(k) in the in-memory indices of the VolatileDB (hash and prev hash) * O(cached epochs * epoch size) in the secondary index caches of the ImmutableDB * ... [SBS]: http://hackage.haskell.org/package/bytestring-0.10.10.0/docs/Data-ByteString-Short.html#g:2
mrBliss
force-pushed
the
mrBliss/short-bytestring
branch
from
dea8236 to
458548a
Compare
edsko
approved these changes
Jul 16, 2020
|
Glad to have this major performance penalty gone finally 😏 |
|
bors ping |
|
pong |
|
bors merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since the Byron and Shelley hash types are
ShortByteStrings under the hood,switch over the representation of
OneEraHashfromStrict.ByteStringtoShortByteStringtoo.To avoid the intermediary
Strict.ByteStringin the conversion, addtoShortRawHashandfromShortRawHashtoConvertRawHash.This avoids the memory overhead that a regular strict
ByteStringhas.According to the Haddocks of
ShortByteString, using aByteString(unshared) for a 32 byte hash (which is the case for Byron andShelley) requires 72 + 32 bytes, but a
ShortByteStringrequires only 32 + 32bytes. This can make a noticeable difference in memory usage as we're keeping
lots of hashes in memory:
ImmutableDB