-
Notifications
You must be signed in to change notification settings - Fork 104
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
multi: optimize MaxOrder #1684
multi: optimize MaxOrder #1684
Conversation
2d1bfd7
to
07b93e7
Compare
// Check if the largest output is too small. | ||
lastUTXO := okUTXOs[len(okUTXOs)-1] | ||
if !isEnoughWith(lastUTXO) { | ||
addUTXO(lastUTXO) | ||
okUTXOs = okUTXOs[0 : len(okUTXOs)-1] | ||
continue | ||
} |
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.
Moving this check up up here was the critical improvement. Before, we iterated the utxos from smallest to largest to find the smallest utxo that satisfied our needs. If none worked, we'd add the last element and try again. But for MaxOrder
, we're using all the utxos, so we'd run the enough function N + (N - 1) + (N - 2) + ... + 1
= N * (N + 1) / 2
, or roughly N²/2
. With the new pattern, it's just N
.
These are smart changes, but I don't think it quite resolves #1612, at least in my case. Initially I thought it must be all this UTXO selection code you're fixing up, but it turned out to be a combination of:
To see what I mean for the first bullet you really need a testnet wallet with hundreds of thousands of utxos. |
Try it out to be sure. I tested with 8000 utxos from the decred simnet wallet, and it shortened the times by a few orders of magnitude. re frontend spamming: I've observed that too, but was unable to reproduce after the fix. I can test some more. |
Yeah it was definitely sitting on listunspent in that issue, although maxSell, not maxBuy. Regarding the frontend metering, I think it would ideally be the kind of request that gets fired after nothing has been typed for say 250 ms, similar to many live search boxes on the internet. Instead, how it behaves is if you want to type out 0.001125 in the rate, each digit pressed (after it becomes a valid non-zero rate) immediately sends the maxsell request, and then it'll be metering by the time you get to "5". |
8da6f2d
to
220494d
Compare
That last push solved the maxSell hammering for me. The maxbuy backend times are also still good. |
This comment was marked as outdated.
This comment was marked as outdated.
Perfection! |
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.
Test well. Leaving up for a bit for other reviews.
@@ -884,8 +885,11 @@ export default class MarketsPage extends BasePage { | |||
this.setMaxOrder(mkt.maxSell.swap) | |||
return | |||
} | |||
if (mkt.maxSellRequested) return |
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 is the critical bit that really resolves #1612 for me
Resolves #1612