Replace unvalidatebitfield to bitfield to avoid validating when state… #617
Conversation
lyswifter
changed the title
|
@Stebalien please give a check for those commits |
Codecov Report
@@ Coverage Diff @@
## master #617 +/- ##
==========================================
+ Coverage 84.42% 84.45% +0.02%
==========================================
Files 88 88
Lines 18083 18051 -32
==========================================
- Hits 15267 15245 -22
+ Misses 2816 2806 -10
|
actors/miner/src/sector_map.rs
Outdated
| @@ -80,7 +80,7 @@ impl DeadlineSectorMap { | |||
| policy, | |||
| deadline_idx, | |||
| partition_idx, | |||
| BitField::try_from_bits(sector_numbers.iter().copied())?.into(), | |||
| &BitField::try_from_bits(sector_numbers.iter().copied()).unwrap(), // sector_numbers may be nil to unwrap | |||
actors/miner/src/sector_map.rs
Outdated
| @@ -106,26 +106,18 @@ impl PartitionSectorMap { | |||
| partition_idx: u64, | |||
| sector_numbers: Vec<u64>, | |||
| ) -> anyhow::Result<()> { | |||
| self.add(partition_idx, BitField::try_from_bits(sector_numbers)?.into()) | |||
| self.add(partition_idx, &BitField::try_from_bits(sector_numbers).unwrap()) | |||
There was a problem hiding this comment.
This can panic if passed a sector number that's out of the bitfield range (u64::MAX). Let's use ?.
|
@Stebalien I fixed them and you can have a look... |
actors/miner/src/sector_map.rs
Outdated
| } | ||
| None => { | ||
| let sector_numbers = sector_numbers.clone(); |
There was a problem hiding this comment.
Sorry, I missed this. Any reason to clone here instead of taking by-value in add?
There was a problem hiding this comment.
&BitField because maybe better to receive a reference of bitfield instead of a value can help to reduce the size of parameters. What do you think?
There was a problem hiding this comment.
In the expected case, we'll have one bitfield per partition. So, if we take it by reference, we'll have to clone it. Cloning is significantly more expensive than copying 36 bytes (the size of a BitField object in wasm).
There was a problem hiding this comment.
All right, I will fix it to pass a value directly.
|
Sorry, one thing I missed. Otherwise LGTM. |
|
@anorth I'll leave this for you to merge in case it conflicts with something else you're working on. |
filecoin-project#617) Co-authored-by: lyswifter <[email protected]>
replace unvalidatebitfield to bitfield