Optimize trade chart view [6] #5794
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chimp1984
force-pushed
the
optimize-trade-chart-view-6
branch
from
dd0202d to
4369db3
Compare
chimp1984
force-pushed
the
optimize-trade-chart-view-6
branch
from
4369db3 to
45a40ab
Compare
|
I'm starting to review from 6cc4265 onwards. |
ripcurlx
previously approved these changes
Nov 9, 2021
Make buildUsdPricesPerTickUnit static and pass params Rename usdPriceMapsPerTickUnit to usdAveragePriceMapsPerTickUnit Rename local variable map to usdAveragePriceMap Move method calls syncPriceFeedCurrency() and setMarketPriceFeedCurrency() before other calls (those will become async later)
use it instead of updateSelectedTradeStatistics (preparation for follow up commit)
Convenience util for CompletableFuture.allOf method
The creation of TradeStatistics3ListItem is rather fast but the applying to the list is due sorting pretty slow (300 ms) as its > 100k items. We do the applying on the callback thread. Seems JavaFx permits that. So we can keep the UI thread unblocked. Remove modelReadyListener Renamed model.selectedTradeStatistics to model.tradeStatisticsByCurrency
in parallel and once both are done we call asyncUpdateChartData (not yet refactored). Clear all data at deactivate This cause a bit of costs when we activate again but as we delegate now all work to threads it should be OK. It decreases the memory usage if we do not keep those data in the fields. The View classes are cached in the view loader so all data in fields stays in memory once it was activated once and not manually cleared in deactivate. Move getTradeStatisticsForCurrency to ChartCalculations Rename buildUsdPricesPerTickUnit to getUsdAveragePriceMapsPerTickUnit Rename selectedTradeStatistics to tradeStatisticsByCurrency Make itemsPerInterval final Remove modelReady Add deactivateCalled flag
…rtCalculations Make maxTicks static and rename to MAX_TICKS
it to table in UI thread afterwards. Chain updateSelectedTradeStatistics and updateChartData calls.
chimp1984
force-pushed
the
optimize-trade-chart-view-6
branch
from
45a40ab to
a659a78
Compare
|
Merged/Rebased with master |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #5790
Starts at ebf5eac
This optimizes performance and memory usage of the Trade Charts view (market).
It uses about 620MB (in JProfiler) when visible and 600 MB when at other view.
Before it was about 680-700 MB and memory did not really go lower when moving away (as we kept the data structures for caching).
It uses CompletableFutures and moved all heavy calculation code to static methods. So at view activation there is nothing heavy left on the UI thread and it can render faster, and as soon the tasks are completed the UI gets updated to render the table and charts.
I think that pattern can be used elsewhere (like BSQ Dashboard which also uses the trade stats and is therefore heavy). But it requires caution to not call JavaFx controls from a non UI thread.
EDIT: Merged and rebased with #5790