huwwp / cryptop

command line crypto portfolio
MIT License
235 stars 41 forks source link

High CPU use (low performance) #21

Closed jfestrada closed 7 years ago

jfestrada commented 7 years ago

I didn't review the code but the CPU is highly used by python3 when the script is running.

Br

jfestrada commented 7 years ago

I've solved it just adding time.sleep(60) in the main loop. But this is not the real solution because the screen will not be refreshed until the 60 seconds will be expired.

image

juanjux commented 7 years ago

22 Should fix this if merged

jfestrada commented 7 years ago

Maybe a good idea could be update the data asynchronous and update the screen every 30 seconds, also should be a good idea to show a count down.

Br

juanjux commented 7 years ago

@jfestrada the PR actually updates the data asynchronously (using threading.Timer instead of async, easier than a loop just for this but eventually the program could benefit from an event loop once more features are added). I also tried to update the screen in that same thread and it works (probably because of the GIL) so I left it that way, trough updating just the data and then retrieving that and updating the interface on the main thread would be better (but not simpler).

huwwp commented 7 years ago

Reopened as the PR had issues with terminal resizing, will look more into it when I get a chance

huwwp commented 7 years ago

Latest build runs under 1% cpu usage in most situations for me, jumps to about 2.5% during requests, let me know how it goes

jfestrada commented 7 years ago

I've tested it and i can confirm that the latest build barely use the CPU.