-
Notifications
You must be signed in to change notification settings - Fork 30
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
Stress tests #43
Stress tests #43
Conversation
Should we also resolve #18 ? |
I think we should do this in a separate PR since I expect that we'll have to move code around quite a bit for this. |
25a6925
to
2f704cf
Compare
Coverage increased (+1.3%) to 90.983% when pulling fb65bbeea409a997f9e06ba270f9d60a0dbcf959 on stress_tests into 2f704cf on master. |
33e9ce8
to
8f1773f
Compare
@@ -14,7 +14,7 @@ const ( | |||
) | |||
|
|||
type SigningKey ed25519.PrivateKey | |||
type VerifKey ed25519.PublicKey | |||
type VerifyKey ed25519.PublicKey |
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 stuff doesn't look like belongs in this pull.
You should git rebase -i master
and d
(drop) that commit.
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.
Should be OK now.
- added benchmarks for creating large directory and doing lookups from large directory - TB test - MakeRand and Digest test
- s/createLargePadForBenchmark/createPad <- could be useful for other tests, too - added tests for some edge cases - removed obsolete check `uint64 < 0` which can't happen and error
- benchmark pad.Update(nil) in tree with 10^5 users (takes on average 0.23 s and consumes 72 MB)
- more stress tests/benchmarks for PAD Update and LookUp (used for pprof)
- test coverage in utils package
t.Fatal(err) | ||
} | ||
|
||
// TODO shouldn't there be a serialize function? |
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.
Can we use this function? https://github.com/coniks-sys/coniks-go/blob/master/merkletree/tb.go#L21-L27
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.
Makes sense! Thanks for the pointer.
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.
Now there is even a public Serialize
method (https://github.com/coniks-sys/coniks-go/blob/master/merkletree/tb.go#L29-L31). I would prefer to use it instead.
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.
Makes sense to me!
Was this done? |
Oops. I definitely did something terrible. Sorry for the noise. |
Please see #54 . I'm so sorry about that :( |
Please, don't worry. Luckily, it easy to revert using git(hub) as you already did :-) |
will resolve #16