flow-go
flow-go copied to clipboard
Remove duplicated top-level `EncodableSnapshot` fields
This PR removes the Head
, LatestSeal
, and LatestResult
fields from EncodableSnapshot
, addressing this outstanding TODO from https://github.com/onflow/flow-go/issues/5666:
Remove additional redundant fields from EncodableSnapshot (in particular LatestSeal, Result, Head). See https://github.com/onflow/flow-go/pull/5682#issuecomment-2080002868
In this implementation, I removed the redundant fields and replaced them with getters which searched for the corresponding data in the SealingSegment
.
- For
Head
, it's very fast and easy to find the data inSealingSegment
- For
LatestSeal
, it's relatively fast and a bit more conceptually complex to find the data. In the worst case we might need to search through one non-empty block payload, and hash several seals. - For
LatestResult
, it's relatively easy, but potentially slow to find the data. In the worst case we might search through many block payloads and hash many results.
These methods are used sparingly on inmem.Snapshot
(everything in the critical path uses badger.Snapshot
, so I don't think the performance regression will have a meaningful impact.
Codecov Report
Attention: Patch coverage is 45.45455%
with 24 lines
in your changes are missing coverage. Please review.
Project coverage is 55.79%. Comparing base (
0436edf
) to head (d9fc06c
).
Additional details and impacted files
@@ Coverage Diff @@
## master #5884 +/- ##
=======================================
Coverage 55.79% 55.79%
=======================================
Files 1128 1128
Lines 88998 89021 +23
=======================================
+ Hits 49652 49665 +13
- Misses 34608 34616 +8
- Partials 4738 4740 +2
Flag | Coverage Δ | |
---|---|---|
unittests | 55.79% <45.45%> (+<0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.