Fix too many open files bug

[miraclx]

Fixes #52

[tarkah]

Change looks good! I'm going to do a bit of testing today with lsof to see what the difference is before and after this change.

[tarkah]

Ok, this change is HUGE. Monitoring the # of open files previously vs this PR:

watch -n1 "ls -al /proc/$(pgrep tickrs)/fd | wc -l"

With 4 tabs open, switching between time frames, I would have 52 files open. This number increases with each new symbol added. With your change, it now averages about 20 open, regardless of how many tabs are created.

Great stuff!

[miraclx]

Glad to have helped, thanks for your work by-the-way.

However, I did notice a slight delay in the asynchronicity of the tasks now that all tasks use a single isahc::HttpClient.

It might be that underneath the isahc::HttpClient handles tasks synchronously and in sequence.

Perhaps engineering a pre-allocated pool of clients that can collectively handle requests independently of each other whilst not spawning new clients for each task would be a better way to move forward.

Do let me know if you notice this on your end, would love to help address it.

[tarkah]

Hmm interesting. I didn't notice anything when testing today, but im not really sure what you mean by slight delay?

[miraclx]

Lemme clarify.

Maybe this is a non-issue, but before, when each task was spawning independent clients, they seemed to resolve a lot faster, sometimes all at once. Now, they seem to resolve one after the other like there's some sort of queuing happening underneath.

[tarkah]

Ahh I see. Let me test with launching bunch of symbols and starting in summary --summary mode before and after to see if I notice any difference. That'd be the easiest way to tell I think

[tarkah]

https://docs.rs/isahc/1.1.0/isahc/#asynchronous-requests

Confirmed to be fully asynchronous. I just tested with 8 symbols in summary mode and they all load instantly.

[tarkah]

The one thing I am going to work on over the next week is hashing the state of my widgets and holding off rendering unless state hash has changed. I noticed CPU usage is quite high when using summary mode with a handful of tickers showing. Since the app gets a redraw request every time new data comes in for any widget, redraws can happen quite often, but maybe only 1 of the widgets received new data within that time frame. So instead of just blindly re-rendering each one, I'll only render ones that have a new hash.

I really want to do some profiling and optimization next.

[tarkah]

Like right now because it's the weekend, the state of my widgets doesn't change AT ALL, so nothing changes on the screen. I'm sitting at ~10% single core CPU usage when I should be able to cut that to basically nothing since there's nothing new to render.

[tarkah]

Its really bad on summary view with 5Y timeframe since there is so much data within each widget so the render is super expensive. 5Y with 8 widgets showing is ~60% usage vs 1D is ~10%

[miraclx]

https://docs.rs/isahc/1.1.0/isahc/#asynchronous-requests

Confirmed to be fully asynchronous. I just tested with 8 symbols in summary mode and they all load instantly.

Yeah, I actually got that by reading the code.

I think I know what the issue is though and it's related to what made me discover #52.

Just sitting there, in summary view of 4 symbols, tickrs averages 23% of CPU utilization.

But when you move quickly through the time frames and finally stop, after a while we're at 65% and the page hasn't resolved with chart data. (it eventually does, but not for a while)

My guess is, when you switch time frames, the requests to gather data for the previous time frame is still active and all those requests resolve before the one you actually want (the current time frame) does

isahc supports canceling requests by dropping, however, so this is fixable

[miraclx]

here's what it looks like at 65% utilization

Normally it shows a loader at the symbol names, but I suspect that disappears because previous requests have resolved but aren't relevant to the current time frame so a chart isn't drawn.

EDIT: just realized the loader is there only while resolving symbol names

[tarkah]

So when you change timeframes, the underlying service drops its async task handle (which will eventually drop the async task). Back when i first implemented this, you couldn't cancel the future by dropping the async task handle. So i had to setup a channel to send a message to the future to let it know to exit, when i dropped the handle. However it wont get that drop message in the loop until after a currently active request finishes.

Now it appears you can cancel futures from dropping the task handle, so I need to update this so it cancels the request IMMEDIATELY.

See here https://github.com/tarkah/tickrs/blob/d056a183660238cc7f850cf1b622c2f0ff377e12/src/task.rs#L41