-
Notifications
You must be signed in to change notification settings - Fork 314
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
fix: hard-code rows to discard #1724
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to be correct
let size = get_base_tree_size::<Tree>(SectorSize(sector_size))?; | ||
let tree_c_config = StoreConfig { | ||
path: PathBuf::from(output_dir.as_ref()), | ||
id: CacheKey::CommCTree.to_string(), | ||
size: Some(size), | ||
rows_to_discard, | ||
rows_to_discard: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If TreeR has discarded rows, shouldn't also TreeC? Admittedly, I don't fully understand rows_to_discard
and its implications
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If TreeR has discarded rows, shouldn't also TreeC? Admittedly, I don't fully understand
rows_to_discard
and its implications
That's exactly the confusing thing, which motivated this change. The rows_to_discard
really only ever applies to TreeR
. Even if it's set, it would be ignored by a TreeC
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To put it another way, we need to keep TreeR around for the long term for PoSt proving, where-as TreeC is removed after the initial PoRep proof usage. The idea was that if we need to keep TreeR around -- how could we optimize (down) the amount of space that it requires on disk (i.e. compaction/level cache store).
Looks good to me too. I just have the one question about TreeC discarded rows, if TreeC has the same number of leaves and arity as TreeR. |
Only rows of the TreeR get discarded. Hence hard-code all other cases to 0 to add clarity, that they don't discard any rows. This makes the code easier to follow and reason about.
3f15aaf
to
3529426
Compare
Due to the rebasing, it makes it hard to review whether anything changes between the last review or not. Easiest way to verify that nothing changed is diffing between the actual changes, e.g.: git show 3f15aaf4c46f260a4a0c88f977ccdf455f204f53 > /tmp/a
git show 3529426a3a241fbd3362b0aa4889b402139201b5 > /tmp/b $ diff /tmp/a /tmp/b
1c1
< commit 3f15aaf4c46f260a4a0c88f977ccdf455f204f53
---
> commit 3529426a3a241fbd3362b0aa4889b402139201b5
12c12
< index dc551681..1cb4af0c 100644
---
> index 4dda1712..a43ac363 100644
15c15
< @@ -10,7 +10,6 @@ use filecoin_proofs::{
---
> @@ -9,7 +9,6 @@ use filecoin_proofs::{
23c23
< @@ -20,7 +19,7 @@ use rayon::prelude::{
---
> @@ -19,7 +18,7 @@ use rayon::prelude::{
32c32
< @@ -194,12 +193,8 @@ pub fn create_replicas<Tree: 'static + MerkleTreeTrait<Hasher = PoseidonHasher>>
---
> @@ -193,12 +192,8 @@ pub fn create_replicas<Tree: 'static + MerkleTreeTrait>(
48c48
< index 1449679a..18398d39 100644
---
> index 5f541feb..1cad7544 100644
51c51
< @@ -15,7 +15,6 @@ use storage_proofs_core::{
---
> @@ -16,7 +16,6 @@ use storage_proofs_core::{
57,59c57,59
< use storage_proofs_porep::stacked::{self, generate_replica_id, PublicParams, StackedDrg};
< pub use storage_proofs_update::constants::TreeRHasher;
< @@ -338,16 +337,7 @@ where
---
> use storage_proofs_porep::stacked::{
> generate_replica_id, PersistentAux, PublicParams, StackedDrg, TemporaryAux,
> @@ -356,16 +355,7 @@ where
78c78
< index 03375e3b..8cd30167 100644
---
> index 69dd3932..59fc1481 100644
81c81
< @@ -156,11 +156,7 @@ where
---
> @@ -158,11 +158,7 @@ where
94c94
< @@ -271,11 +267,7 @@ where
---
> @@ -273,11 +269,7 @@ where
107c107
< @@ -1179,7 +1171,6 @@ where
---
> @@ -1202,7 +1194,6 @@ where
115c115
< @@ -1191,7 +1182,7 @@ where
---
> @@ -1214,7 +1205,7 @@ where
124c124
< @@ -1234,13 +1225,12 @@ where
---
> @@ -1257,13 +1248,12 @@ where
139c139
< @@ -1252,7 +1242,7 @@ where
---
> @@ -1275,7 +1265,7 @@ where
208c208
< index 1ba2baf8..251d4eca 100644
---
> index 3e5b1035..212f45d6 100644
220,222c220,222
< self, LayerChallenges, PrivateInputs, PublicInputs, SetupParams, StackedBucketGraph,
< - StackedDrg, TemporaryAuxCache, BINARY_ARITY, EXP_DEGREE,
< + StackedDrg, TemporaryAuxCache, EXP_DEGREE,
---
> LayerChallenges, PrivateInputs, PublicInputs, SetupParams, StackedBucketGraph, StackedDrg,
> - TemporaryAux, TemporaryAuxCache, BINARY_ARITY, EXP_DEGREE,
> + TemporaryAux, TemporaryAuxCache, EXP_DEGREE, |
Only rows of the TreeR get discarded. Hence hard-code all other cases to 0 to add clarity, that they don't discard any rows. This makes the code easier to follow and reason about.