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

Speed up trades charts view load #3828

Merged
merged 2 commits into from Jan 2, 2020

Conversation

@stejbac
Copy link
Contributor

stejbac commented Dec 23, 2019

Use a LinkedHashMap in place of a List, for the caching CurrencyUtil fields allSortedFiatCurrencies & allSortedCryptoCurrencies, using the same iteration order as before. In this way, we can avoid a linear search in the lookup methods getFiatCurrency & getCryptoCurrency.

In particular, this speeds up the activation of TradesChartsView (and to a lesser extent OfferBookChartView), which make a lot of calls to CurrencyUtil.getTradeCurrency in the fillTradeCurrencies/updateChartData methods respectively.

Additionally, do some tiding of TradeChartsViewModel and remove some unused constructor-injected fields from that class and others.

To reproduce the hotspot, one can select the Trades tab under Market at least once, then flipping repeatedly between (say) the Market and Buy BTC panels, the following hotspots are revealed by JProfiler:

Screenshot from 2019-12-23 23-01-07

(The also very significant com.sun.javafx.collections.VetoableListDecorator.setAll hotspot seen at the bottom of the screenshot occurs during the initialisation of many views, but I haven't been able to find the cause or a fix for that yet.)

stejbac added 2 commits Dec 22, 2019
These are mostly injected objects that are now redundant, such as some
CoinFormatter and Preferences fields.

Also do some additional minor tidying of TradesChartsViewModel.
Use a LinkedHashMap in place of a List, for the caching CurrencyUtil
fields 'allSortedFiatCurrencies' & 'allSortedCryptoCurrencies', using
the same iteration order as before. In this way, we can avoid a linear
search in the lookup methods getFiatCurrency & getCryptoCurrency.

In particular, this speeds up the activation of TradesChartsView (and to
a lesser extent OfferBookChartView), which make a lot of calls to
CurrencyUtil.getTradeCurrency in the fillTradeCurrencies/updateChartData
methods respectively.
@stejbac stejbac requested review from ripcurlx and sqrrm as code owners Dec 23, 2019
@ripcurlx

This comment has been minimized.

Copy link
Member

ripcurlx commented Dec 27, 2019

Just a heads up that I haven't planed to be available until 2nd of Jan, but I'll still try to do some reviews tomorrow.

@sqrrm
sqrrm approved these changes Jan 2, 2020
Copy link
Member

sqrrm left a comment

ACK

Can't say if it's faster when running it now but it probably is. Still works and removing cruft is good.

@sqrrm sqrrm merged commit 3a74e74 into bisq-network:master Jan 2, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ripcurlx ripcurlx added this to the v1.2.5 milestone Jan 2, 2020
@stejbac stejbac mentioned this pull request Jan 12, 2020
@sqrrm sqrrm mentioned this pull request Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.