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

Increase maximum value of the p cmdline parameter #383

Closed
wants to merge 1 commit into from
Closed

Increase maximum value of the p cmdline parameter #383

wants to merge 1 commit into from

Conversation

matjon
Copy link
Contributor

@matjon matjon commented Mar 19, 2024

The upper limit of the value of the -p cmdline parameter is 32, which is relatively low. Currently, hashing with logN=20, r=8 and p=32 takes around 90s on my slow CPU (Intel i3-7020U).

This is in contrast to the --logN parameter, which can be set up to 40, which corresponds to 1TB of RAM. Additionally, the program will automatically pick high values of p when given high values of the -t (maxtime) parameter.

So allow specifying higher values of the p parameter in cmdline.

The upper limit of the value of the `-p` cmdline parameter is 32, which
is relatively low. Currently, hashing with logN=20, r=8 and p=32 takes
around 90s on my slow CPU (Intel i3-7020U).

This is in contrast to the `--logN` parameter, which can be set up to 40,
which corresponds to 1TB of RAM. Additionally, the program will
automatically pick high values of `p` when given high values of the
`-t` (maxtime) parameter.

So allow specifying higher values of the `p` parameter in cmdline.

Signed-off-by: Mateusz Jończyk <[email protected]>
@gperciva
Copy link
Member

Hmm. Here's some related history, to save reviewers time digging it up themselves:

$ ./scrypt enc -v -t 1000 config.h config.enc
Please enter passphrase: 
Please confirm passphrase: 
Parameters used: N = 524288; r = 8; p = 204;
    Decrypting this file requires at least 536 MB of memory (899 MB available),
    and will take approximately 996.0 seconds (limit: 1000.0 seconds).
^C

@cperciva
Copy link
Member

cperciva commented Apr 7, 2024

Yeah, this is fine. Most people won't need values > 32, but it's ok to make them available.

@gperciva Can you kick the checks to run again so I can merge this?

@gperciva
Copy link
Member

gperciva commented Apr 7, 2024

I put this PR into #385 so that the actions would run.

(According to my reading of the github docs, we're supposed to have an "Workflow(s) awaiting approval" visible to us admins, but we don't have one. I spent a while looking into this last Sep, but no success.)

@gperciva
Copy link
Member

gperciva commented May 9, 2024

This was merged as #385.

@gperciva gperciva closed this May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants