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 DAO state monitor view load #4035

Merged

Conversation

stejbac
Copy link
Contributor

@stejbac stejbac commented Mar 9, 2020

Compute height cells lazily to speed up DaoStateMonitorView

Avoid a bottleneck computing the cycle index & calling Res.get(..) for every block since genesis in the DAO state monitor view, when building the DaoStateBlockListItem objects, by making the height field lazy. To do this, pass the cycle index into the constructor using an IntSupplier and make the height a memoised Supplier<String> with a custom getter.

Also add a unit test to check that the auto-generated equals & hashCode methods still work as expected, as it isn't totally clear what Lombok would do when a field type differs from its getter return type.

--

Flippling back and forth between the Network monitor tab and the Governance tab revealed the following call tree in JProfiler:

Screenshot from 2020-03-06 10-16-21

As can be seen above, most of the time is spent in the stream pipeline collecting new DaoStateBlockListItem objects, in DaoStateMonitorView.onDataUpdate. After the change, the main bottleneck appeared to be from internal JavaFX code, in the listItems.setAll call in onDataUpdate (perhaps from one of the listeners on DaoStateMonitorView.sortedList). However, I believe there was a noticeable speedup. Also, the slowdown over time is likely to be quadratic without the change, as the number of cycles and block grows linearly. (The main bottleneck appears to be computing the cycle index of each block.)

stejbac added 2 commits March 6, 2020 17:03
Avoid a bottleneck computing the cycle index & calling 'Res.get(..)' for
every block since genesis in the DAO state monitor view, when building
the DaoStateBlockListItem objects, by making the 'height' field lazy. To
do this, pass the cycle index into the constructor using an IntSupplier
and make the height a memoised 'Supplier<String>' with a custom getter.

Also add a unit test to check that the auto-generated equals & hashCode
methods still work as expected, as it isn't totally clear what Lombok
would do when a field type differs from its getter return type.
@ripcurlx ripcurlx requested a review from ManfredKarrer March 9, 2020 11:25
@ripcurlx
Copy link
Contributor

ripcurlx commented Mar 9, 2020

Waiting for ACK by @ManfredKarrer as it touches the DAO package

@ManfredKarrer
Copy link
Contributor

ManfredKarrer commented Mar 9, 2020

@ripcurlx It is only touching a view class, so I do not need to ACK that (only core dao package). Had a quick look and looks good to me, but don't consider it a review....

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

ACK

Tested it on Mainnet and everything works as before. Code looks fine.

@ripcurlx ripcurlx merged commit dc583d1 into bisq-network:master Mar 10, 2020
@ripcurlx ripcurlx added this to the v1.2.8 milestone Mar 10, 2020
@ripcurlx ripcurlx added the is:priority PR or issue marked with this label is up for compensation label Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is:priority PR or issue marked with this label is up for compensation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants