Skip to content
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

Reduce API calls from certain elements by keeping global state up-to-date and caching redundant data #126

Merged
merged 13 commits into from
Apr 28, 2020

Conversation

pinheadmz
Copy link
Contributor

@pinheadmz pinheadmz commented Apr 15, 2020

Closes #100

This PR reduces API calls in certain views to improve UI performance. The "Your Bids" view in particular has a table with three elements per bid: each of these elements was individually calling getNameInfo() on the same name! In addition, the use of componentWillMount() in certain elements to update state is insufficient since a new block event (and state update) will not trigger this method. The elements themselves should just rely on the global state being updated already.

Since bids and namestates only change when new blocks connect to the chain, we only really need to gather this data once per block. The data is already stored in the global state, so all we have to do is change when it is updated.

This patch adds a new function onNewBlock() to backgroundMonitor which is executed any time a doPoll() encounters a new block from hsd. This function gets current fee rates, gets all bids, and then iterates through all the bids getting updated namestates. This means that on every new block, UI may be a bit sluggish for a minute. A new progress indicator is added in the sidebar footer.

Tested by running a bash script that opened 300 names and then bid once or twice on each name.

Future work:

  • Add pagination to hsd wallet (see Wallet TX count and time indexing bcoin-org/bcoin#605)
  • Add functionality either in Bob or hsd to filter out expired auctions or bids that "don't matter anymore" for one reason or another.
  • Switch the hsd<->Bob link from 10-second polling to websocket events. If possible, this will keep Bob up to date with essentially zero lag.
  • Explore getNameInfo for ways to reduce 2 API calls (node.getNameInfo + wallet.getAuctionInfo) to one, or cache some data if possible.

Demonstration

UI elements seem more responsive than before even when a new block is being processed. Once the new block processing is done, everything is perfectly snappy since no new API calls are made when loading/changing views.

Note the new "New Block Status" message just above the current height in the sidebar footer.

LoadBidsOnNewBlock

@pinheadmz pinheadmz marked this pull request as ready for review April 15, 2020 18:14
@pinheadmz pinheadmz requested a review from mslipper April 15, 2020 18:14
@pinheadmz
Copy link
Contributor Author

rebase to 525a7e5:
rebasing on master

@mslipper
Copy link
Contributor

https://www.youtube.com/watch?v=VI6dsMeABpU. Will review in the AM.

@pinheadmz
Copy link
Contributor Author

TODO: Use newBlockStatus to monitor the flurry of getNameByHash calls on boot

@mslipper
Copy link
Contributor

LGTM. Let me know when you want a merge.

TXs are still sorted by date but this way we can key into the TX
history by txid and avoid requesting data we already have.
@pinheadmz
Copy link
Contributor Author

@mslipper added one last patchset. This replaces the state.wallet.transactions array with a Map. The Map is still sorted by date, and gives us the feature of being able to test it for a certain txid. With that in place, another round of redundant API calls can be avoided.

Pushing this up for now, going to bang on it a bit more tomorrow and make sure it's all good.

@@ -56,7 +56,7 @@ export default function walletReducer(state = getInitialState(), {type, payload}
...state.balance
},
isLocked: true,
transactions: []
transactions: new Map()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious: what is the advantage of using a Map in this context as opposed to a simple {}?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PropTypes checker was throwing an error before the first update.

@pinheadmz
Copy link
Contributor Author

rebase to 216a858

Remove leftover console.log from debugging and force-push

@mslipper mslipper merged commit 46730f9 into kyokan:master Apr 28, 2020
@pinheadmz pinheadmz deleted the processblock1 branch July 1, 2022 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Performance] Bids and Domains take a very long (>5mins) time to load
2 participants