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

CPU consumption skyrockets on create offer / take offer #4649

Closed
oscarguindzberg opened this issue Oct 14, 2020 · 15 comments · Fixed by #5090
Closed

CPU consumption skyrockets on create offer / take offer #4649

oscarguindzberg opened this issue Oct 14, 2020 · 15 comments · Fixed by #5090
Labels

Comments

@oscarguindzberg
Copy link
Contributor

oscarguindzberg commented Oct 14, 2020

Description

CPU consumption skyrockets on create offer / take offer.

Version

1.4.0

Steps to reproduce

Option 1: Create offer

  • Click "Create new offer to buy btc"
  • Trading account: ETH...
  • Enter any values
  • Click next step

Option 2:Take offer

  • Find an offer to sell btc (buy eth)
  • Take the offer
  • Click next

Expected behaviour

CPU consumption at normal level.

Actual behaviour

CPU consumption skyrockets. Computer performance is degraded. Java process uses 70% of the CPU.
Once I exit that screen, CPU consumption goes back to normal.

Screenshots

Device or machine

Mac OSX Catalina

Additional info

Tested on regtest connected to a localhost bitcoin core
Just to be clear, the problem is on the screen that has the "Transfer funds from bisq wallet" and "open your external wallet for funding" buttons.

@ghost
Copy link

ghost commented Oct 14, 2020

Same thing happens on Ubuntu. Also, it happens on v1.3.7 mainnet as well.

Before that screen, all CPU @ 5%. On the screen mentioned, one CPU goes up to 50% and the others all go up to around 20% (from previous values @ 5%). Cancel out from that screen and they all drop down to 5% again. (Using htop to measure).

I wonder if it is the animated icon... 🤔
.. and yes that's it. Comment out 3 occurrences of waitingForFundsSpinner.play() in MutableOfferView.java and then the CPU stays at 5%.

@wiz
Copy link
Contributor

wiz commented Oct 14, 2020

Sounds like GPU is not being used to render that animation

@chimp1984
Copy link
Contributor

The spinner always had a performance issue. So that might be the reason. I think we moved to diff. buys animations elsewhere. @ripcurlx can you comment? I am not so much up to date with UI.
Another reason could be fee estimation, but a breakpoint there should easily reveal if thats the case.

@chimp1984
Copy link
Contributor

Best to test to just deactivate the waitingForFundsSpinner and see if that solves it.

@wiz
Copy link
Contributor

wiz commented Oct 15, 2020

@chimp1984 yes, @jmacxx confirmed it was the spinner in his comment:

Comment out 3 occurrences of waitingForFundsSpinner.play() in MutableOfferView.java and then the CPU stays at 5%.

@chimp1984
Copy link
Contributor

Ah sorry was not reading well ;-).

We created our own TxConfidenceIndicator for the same reason. BusyAnimation extends JFXSpinner. And JFXSpinner extends ProgressIndicator. ProgressIndicator is known to be terrible with CPU waste. Earlier it was BusyAnimation extends ImageView. I guess we need to kick out those ProgressIndicator implementations.

@chimp1984
Copy link
Contributor

Was a very issue and was kind of resolved but seems it got re-introduced with the redesign:
#485

@cd2357 cd2357 added the in:gui label Oct 15, 2020
@cd2357
Copy link
Contributor

cd2357 commented Oct 15, 2020

Might be improved by setting https://docs.oracle.com/javase/8/javafx/api/javafx/scene/Node.html#cacheProperty on these spinner and progress indicators.

(For reference, see Did we try turning cache to true and cache hint to SPEED? in https://wiki.openjdk.java.net/display/OpenJFX/Performance+Tips+and+Tricks. They're optimizing animated nodes in a simple JavaFX game, but same principles probably apply here too)

@ghost
Copy link

ghost commented Oct 16, 2020

@cd2357 I added setCache(true) and setCacheHint(CacheHint.SPEED) to various places in class BusyAnimation (the constructors & the play method) but no change in behavior. CPU 2 always goes over 60% and stays there as long as the animation is shown.

Spinner being played:
cpu-spinner

No spinner:
cpu-nospinner
cpu-no-spinner2

@stale
Copy link

stale bot commented Jan 17, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the was:dropped label Jan 17, 2021
@ghost
Copy link

ghost commented Jan 18, 2021

Still relevant.

I propose removing the animated circle shown when waiting for funds:

image

And the animated inchworm shown when parsing a new DAO block:

image

@stale stale bot removed the was:dropped label Jan 18, 2021
@chimp1984
Copy link
Contributor

Agree. Lets remove them (but keep code, just comment out/deactivate). Once we find a dedicated UI dev we might fix the performance issue or find an alternative.

@cd2357
Copy link
Contributor

cd2357 commented Jan 21, 2021

Are we sure these animations are CPU intensive (and removing them reduces CPU load)?

Its unfortunate they were removed, because they were very useful.

@ghost
Copy link

ghost commented Jan 24, 2021

The CPU hit I documented above was quite conclusive. Apparently it impacts some users more than others, for example even with the CPU pegged at 70% the app still works fine for me. But for others it makes Bisq unusable. It would be nice to get the animations back, and I tagged the disabled code with the ID of this issue so it is trivial to revert.

I propose we ask @BawdyAnarchist, @crocket and Oscar if 1.5.5 is an improvement. I'll ping them after 1.5.5 is released. If we get no positive feedback I'll re-enable the animations.

@ripcurlx
Copy link
Contributor

I would also be interested if using the latest JavaFX version as in #4242 makes a difference for this users, when having the animations enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants