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

Extracting (decrypting?) 6.3x PSARs is frigging slow #14

Closed
artart78 opened this issue Feb 11, 2021 · 5 comments · Fixed by #27
Closed

Extracting (decrypting?) 6.3x PSARs is frigging slow #14

artart78 opened this issue Feb 11, 2021 · 5 comments · Fixed by #27

Comments

@artart78
Copy link
Contributor

Why???

@John-K
Copy link
Owner

John-K commented Feb 13, 2021

Did some quick profiling:
6.31:

(pprof) top
Total: 24998 samples
   19005  76.0%  76.0%    23452  93.8% bn_mon_muladd_dig
    4132  16.5%  92.6%     4132  16.5% bn_sub_1
     512   2.0%  94.6%    24089  96.4% bn_mon_mul

6.61:

(pprof) top
Total: 499 samples
     133  26.7%  26.7%      160  32.1% bn_mon_muladd_dig
      98  19.6%  46.3%       98  19.6% read_bit
      44   8.8%  55.1%       44   8.8% rijndaelEncrypt

So, 2x the magnitude of bignum operations.

I bet something weird is going on with how we enumerate files causing us to visit files multiple times? Might need to instrument things to figure it out.

To reproduce:

  • link with -lprofile
  • run CPUPROFILE=631.pprof psardumper 631.PBP
  • view results pprof psardumper 631.pprof

@artart78
Copy link
Contributor Author

bignum functions are only used for ECDSA checking IIRC, was ECDSA checking enabled for all modules in 6.3x but removed in 6.6x?

@artart78
Copy link
Contributor Author

I tried commenting ecdsa_verify, and indeed it becomes a lot faster. It could probably be disabled by default since I doubt it's functional anyway.

@John-K
Copy link
Owner

John-K commented Feb 24, 2021

Might be nice to add a flag to enable that behavior and have it disabled by default?

@artart78
Copy link
Contributor Author

Yep, related to #13, I'm even considering disabling it for now as I doubt it works...

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 a pull request may close this issue.

2 participants