Cache results in account witness domain #4953
Conversation
|
Here are results from performance tests (based on live network data): I used a loop for 5 iterations over each offer. For findWitness 10000 iterations. Version of that PR: Master version: I expected that the findWitness call is more expensive but seems thats not the bottleneck. So the optimization with I think it requires further investigations where the bottlenecks are in the tested methods. Overall that PR brings still about a 20-30% improvement. As its a bit risky domain I have no strong opinion about to merge that PR or close it and delay for a later more broad investigation and improvement. Up to maintainers.... |
|
Seems some Travis issue again: "Could not resolve com.google.api.grpc:proto-google-common-protos:1.12.0." Will force push... |
The accountAgeWitnessMap is very large (70k items) and access is a bit expensive. We usually only access less than 100 items, those who have offers online. So we use a cache for a fast lookup and only if not found there we use the accountAgeWitnessMap and put then the new item into our cache.
The getSignedWitnessSet is called very often and is a bit expensive. We cache the result in that map but we remove the cache entry if we get a matching SignedWitness added to the signedWitnessMap.
directly and we did not remove the item from the cache) - Add getSignedWitnessMapValues method for access to signedWitnessMap values - Remove Getter for signedWitnessMap - Add removeSignedWitness method (used in test only) - Use addToMap in test instead of direct access to map - Remove getSignedWitnessSetCache entry matching signedWitness.getHashAsByteArray() as well. See comment. @sqrrm: Can you cross check?
As the normal maps are not using a ConcurrentHashMap it is likely unnecessary as well that I am not aware of a multi-threaded access. But as it does not show any difference in performance it is likely the bit more safe option.
chimp1984
force-pushed
the
cache-results-in-account-witness-domain
branch
from
71f3671 to
f2273e6
Compare
|
Travis has issues again... happens quite often recently ;-( Will force push... |
core/src/main/java/bisq/core/account/sign/SignedWitnessService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/bisq/core/account/sign/SignedWitnessService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/bisq/core/account/sign/SignedWitnessService.java
Outdated
Show resolved
Hide resolved
|
@chimp1984 I'm waiting for your fixes before merging |
Change comments (after understanding the domain usage better) Maintain the set at add and remove methods Use HashMap instead of ConcurrentHashMap as that class is only used from UserThread and other fields are not threadsafe either.
anymore as we maintain the signedWitnessSetByAccountAgeWitnessHash at each add/remove operation.
iterations over getSignedWitnessMapValues
There was a problem hiding this comment.
utACK
@chimp1984 did you want to add anything else to this PR?
No description provided.