Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use snappy-framed format for compressing bellatrix+ database entries #3551

Merged
merged 3 commits into from
Mar 29, 2022

Conversation

arnetheduck
Copy link
Member

.era files and Req/Resp protocols use framed formats - aligning the
database with these makes for less recompression work overall as gossip
is sent only once while req/resp repeats (potentially) - this also
allows efficient pruning-to-era where snappy-recompression is the major
cycle thief.

db.blocks[BeaconBlockFork.Phase0].get(key.data, decode).expectDb() and success or
db.v0.getPhase0BlockSSZ(key, data)

proc getBlockSZ*(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are getBlockSZ(phase0) and getBlockSZ(altair) separate functions? Bellatrix differs in that it does assign rather than snappy.decode.

Copy link
Member Author

Choose a reason for hiding this comment

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

phase0 reads from an additional table.. yeah, there's better ways to factor the code I'm sure but ..

# TODO rollback is needed to deal with bug - use `noRollback` to ignore:
# https://github.com/nim-lang/Nim/issues/14126
# TODO RVO is inefficient for large objects:
# https://github.com/nim-lang/Nim/issues/13879
Copy link
Contributor

Choose a reason for hiding this comment

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

Is RVO still inefficient for large objects? That Nim issue is closed.

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, yes.. there's nim-lang/Nim#15571 now instead

Copy link
Member Author

Choose a reason for hiding this comment

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

longer story would be that to remove the TODO, one would need to examine the C code again and see what's going on - issue might be closed but that doesn't mean it's actually fixed for our use case

@arnetheduck arnetheduck force-pushed the snappy-frame-db branch 2 times, most recently from 80e7f9f to 23f9abf Compare March 28, 2022 11:26
@arnetheduck arnetheduck changed the title use snapy-framed format for compressing bellatrix+ database entries use snappy-framed format for compressing bellatrix+ database entries Mar 28, 2022
@github-actions
Copy link

github-actions bot commented Mar 28, 2022

Unit Test Results

     12 files  ±0     834 suites  ±0   1h 3m 47s ⏱️ + 6m 58s
1 686 tests ±0  1 638 ✔️ ±0    48 💤 ±0  0 ±0 
9 825 runs  ±0  9 713 ✔️ ±0  112 💤 ±0  0 ±0 

Results for commit 05fd6d8. ± Comparison against base commit 46e5175.

♻️ This comment has been updated with latest results.

`.era` files and Req/Resp protocols use framed formats - aligning the
database with these makes for less recompression work overall as gossip
is sent only once while req/resp repeats (potentially) - this also
allows efficient pruning-to-era where snappy-recompression is the major
cycle thief.
@arnetheduck arnetheduck enabled auto-merge (squash) March 29, 2022 07:14
@arnetheduck arnetheduck merged commit 5092fc4 into unstable Mar 29, 2022
@arnetheduck arnetheduck deleted the snappy-frame-db branch March 29, 2022 11:33
@arnetheduck arnetheduck restored the snappy-frame-db branch March 29, 2022 12:52
@arnetheduck arnetheduck deleted the snappy-frame-db branch March 29, 2022 12: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