joinmarket-clientserver
joinmarket-clientserver copied to clipboard
JoinMarket-Org
JoinMarket becomes slow with old wallets with a lots of tx history
Especially with wallet-tool methods (summary and especially history). Have noticed it long time ago, never managed to prioritize to find out what's happening there (although #851 was minor improvement). But it definitely shouldn't be so slow. Here is comment from user in reddit (he's running RPi, but that's not the issue, I have noticed the same on more powerful machines).
History takes a very long time, and I assumed it is because there are hundreds of transactions (I run yg). Summary and display also take a long time. Perhaps, that is also because there have been many transactions.
I timed summary and it took 2:34.
Tu be clear what is old and a lots of tx history - I mean yg wallets running for years.
Yes, but also note that it's significantly slower if fidelity bonds are enabled. (In fact before they got merged into a release, they were even maybe 5-10 times slower; it's a function of how many time intervals you scan).
I would generally caution users to shift to a new wallet after the wallet indices reach the several hundreds in each branch, or perhaps every 6 months or so, depending on circumstances.
As for making it faster, I'm not sure. There have been several initiatives to make both syncing and monitoring as fast as is reasonable over the years, but for sure always more can be done.
From a quick profiling, it seems most of the runtime comes from low level bitcointx functions and classes. Does it make sense?
Maybe there's some more room for caching the results of those functions, or to call them less frequently.
The wallet used is a Signet one with around 100 txs (CJ and non) done over its lifetime.
History
Summary
Display
The graph are made using https://github.com/jrfonseca/gprof2dot and cProfile:
python -m cProfile -o output.pstats path/to/your/script arg1 arg2
gprof2dot.py -f pstats output.pstats | dot -Tpng -o output.png
From a quick profiling, it seems most of the runtime comes from low level bitcointx functions and classes. Does it make sense?
Yes, and thanks for that. I may need to ask you about some details of how the percentages apply to each function there, but this could be really useful. I did some basic profiling a while back but it was harder to read.
One thing that pops up in my mind is, how often are we repeating (if at all) the bip32 derivations of keys and perhaps addresses; if we are, we could and should of course cache to ensure that never happens.
So, from the README:
Output
A node in the output graph represents a function and has the following layout:
+------------------------------+
| function name |
| total time % ( self time % ) |
| total calls |
+------------------------------+
where:
- total time % is the percentage of the running time spent in this function and all its children;
- self time % is the percentage of the running time spent in this function alone;
- total calls is the total number of times this function was called (including recursive calls).
An edge represents the calls between two functions and has the following layout:
total time %
calls
parent --------------------> children
Where:
- total time % is the percentage of the running time transfered from the children to this parent (if available);
- calls is the number of calls the parent function called the children.
One thing that pops up in my mind is, how often are we repeating (if at all) the bip32 derivations of keys and perhaps addresses;
If I read the graphs correctly, our common main entry point to these low level functions is from wallet.get_script_from_path(), which indeed is related to that, right? IDK, it could be that we have multiple functions of ours that call that, rather than doing it once, but I'm not familiar enough with the low level stuff to judge it.
Not sure when, but I'll do a review and if I can see clearly that there are repeated calls to the same derivations, I'll just make some kind of cache object for them, and we can re-do the profile test case and see if it makes a difference (bit vague I know, but, anyway).
(I should jot down here that BIP32Wallet does indeed have a cache in the form of a _script_map var but that may not be the full story here).