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

feat: add some README notes on available settings #1177

Merged
merged 3 commits into from
Jun 25, 2020
Merged

Conversation

cryptonemo
Copy link
Collaborator

Addresses #1172

@cryptonemo
Copy link
Collaborator Author

Review feedback/corrections for docs is most useful as a commit/update to this branch.

porcuquine
porcuquine previously approved these changes Jun 18, 2020
Copy link
Collaborator

@porcuquine porcuquine left a comment

Choose a reason for hiding this comment

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

Should there be something here for the parents cache?

@cryptonemo
Copy link
Collaborator Author

Should there be something here for the parents cache?

Sure, please add!

@porcuquine
Copy link
Collaborator

@dignifiedquire Can you add a short description for the parents cache, including any constraints or considerations controlling how that value should be set?

@cryptonemo cryptonemo force-pushed the readme-updates branch 3 times, most recently from 57deff4 to 51ae3e7 Compare June 24, 2020 14:56
porcuquine
porcuquine previously approved these changes Jun 24, 2020
README.md Outdated

### Memory
A related setting that can also be tuned is the SDR parents cache size. This value is defaulted to 2048 nodes, which is the equivalent of 384KiB of resident memory (where each cached node is (6 * 32) = 192 bytes in length). Given that the cache is now located on disk, it is memory mapped when accessed in window sizes related to this variable. This default was chosen to minimize memory while still allowing efficient access to the cache. If you would like to experiment with alternate sizes, you can modify the environment variable
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dignifiedquire Can you check that my sizes above are sane? I pulled those out from looking at the code, but there was a comment you made elsewhere that indicated each cached node was 56 bytes (i.e. using 112KiB RAM), so I'm looking for clarification/corrections here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for clarifying, it's been updated.

@@ -17,6 +18,7 @@ use storage_proofs_core::{
use super::graph::{StackedGraph, DEGREE};

/// Path in which to store the parents caches.
pub const PARENT_CACHE_ENV_VAR: &str = "FIL_PROOFS_PARENT_CACHE";
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use settings.rs for this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, please.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I went with the existing approach taken by the PARAMETER_CACHE

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But can move them both to settings

feat: update pairing dependency
feat: move parameter and parent cache path to settings
@cryptonemo cryptonemo merged commit dd19606 into master Jun 25, 2020
@cryptonemo cryptonemo deleted the readme-updates branch June 25, 2020 20:49
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.

3 participants