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.

Sign up for GitHub

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

Jump to bottom

Large enum variant in store #1713

Closed
paulhauner opened this issue Oct 3, 2020 · 1 comment
Closed

Large enum variant in store #1713

paulhauner opened this issue Oct 3, 2020 · 1 comment
Assignees
@michaelsproul
Labels
database

Comments

@paulhauner
Copy link
Member

Description

There is an enum in the store which contains variants with largely varying sizes:

lighthouse/beacon_node/store/src/lib.rs

Lines 121 to 130 in d9f4819

/// Reified key-value storage operation. Helps in modifying the storage atomically.
/// See also https://github.com/sigp/lighthouse/issues/692
#[allow(clippy::large_enum_variant)]
pub enum StoreOp<'a, E: EthSpec> {
PutBlock(SignedBeaconBlockHash, SignedBeaconBlock<E>),
PutState(BeaconStateHash, Cow<'a, BeaconState<E>>),
PutStateSummary(BeaconStateHash, HotStateSummary),
DeleteBlock(SignedBeaconBlockHash),
DeleteState(BeaconStateHash, Slot),
}

Eyeballing this, it seems that clippy is right and we should definitely Box the block and possible the Cow<BeaconState> (I'm not sure if the Cow needs boxing).
@paulhauner paulhauner mentioned this issue Oct 3, 2020
Closed
@michaelsproul michaelsproul self-assigned this Oct 20, 2020
@michaelsproul
Copy link
Member

I've fixed this while working on #800

@michaelsproul michaelsproul mentioned this issue Oct 21, 2020
Closed
@bors bors bot closed this as completed in acd49d9 Oct 23, 2020
bors bot pushed a commit that referenced this issue Oct 25, 2020
@paulhauner
Address clippy lints, panic in ssz_derive on overflow (#1714)
f157d61 
## Issue Addressed

NA

## Proposed Changes

- Panic or return error if we overflow `usize` in SSZ decoding/encoding derive macros.
  - I claim that the panics can only be triggered by a faulty type definition in lighthouse, they cannot be triggered externally on a validly defined struct.
- Use `Ordering` instead of some `if` statements, as demanded by clippy.
- Remove some old clippy `allow` that seem to no longer be required.
- Add comments to interesting clippy statements that we're going to continue to ignore.
- Create #1713

## Additional Info

NA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michaelsproul michaelsproul

Labels
database
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Merged by Bors] - Implement database temp states to reduce memory usage sigp/lighthouse
2 participants
@michaelsproul @paulhauner