Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Getting test public keys is really expensive on debug builds #3385

Open
eskimor opened this issue Jun 30, 2021 · 4 comments
Open

Getting test public keys is really expensive on debug builds #3385

eskimor opened this issue Jun 30, 2021 · 4 comments
Labels
I4-annoyance Code behaves within expectations, however this “expected behaviour” itself is at issue. I5-tests Tests need fixing, improving or augmenting. I10-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task.

Comments

@eskimor
Copy link
Member

eskimor commented Jun 30, 2021

This innocently looking line of code takes around 700ms on my machine on a debug build.

We should quickly check (if it is not obvious) why it takes that long and if there is nothing to do about it, provide caching via lazy_static as this would quite likely speed up test suits quite a bit.

@eskimor eskimor added I5-tests Tests need fixing, improving or augmenting. I10-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. I4-annoyance Code behaves within expectations, however this “expected behaviour” itself is at issue. labels Jun 30, 2021
@bkchr
Copy link
Member

bkchr commented Jun 30, 2021

This is already a lazy_static 🤔

@burdges
Copy link
Contributor

burdges commented Jun 30, 2021

As a rule, deserializing a public key is never free because it involves operations like this inverse square root https://docs.rs/curve25519-dalek/3.1.0/src/curve25519_dalek/ristretto.rs.html#284 in sr25519, or much worse subgroup checks for pairing friendly curves. Is the .into() perhaps deserializing?

@eskimor
Copy link
Member Author

eskimor commented Jun 30, 2021

Thanks @burdges ! @bkchr - haha, you are right! It is still slow as .... It likely is the into() call, that is slow.

@burdges
Copy link
Contributor

burdges commented Jun 30, 2021

After creation you could keep the keys in a deserialized state then somehow I guess, maybe breaks the key store abstraction somehow, but maybe some caching layer.

If this is what's happening then it'll get worse with the grandpa BLS key, which incurs an on-curve tests that takes much longer.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I4-annoyance Code behaves within expectations, however this “expected behaviour” itself is at issue. I5-tests Tests need fixing, improving or augmenting. I10-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task.
Projects
None yet
Development

No branches or pull requests

3 participants