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

In the UI, calculate bandwidth for the past N seconds rather than just 1 #14

Closed
imsnif opened this issue Nov 23, 2019 · 10 comments
Closed

In the UI, calculate bandwidth for the past N seconds rather than just 1 #14

imsnif opened this issue Nov 23, 2019 · 10 comments

Comments

@imsnif
Copy link
Owner

@imsnif imsnif commented Nov 23, 2019

Right now, the UI works by looping every second, displaying the bandwidth utilization and then clearing it. This gives us an accurate and no-fuss bandwidth-per-second display, but also provides for a less-than-ideal user experience, since utilization information can flicker in and out if it is short lived.

A possible solution would be to log the bandwidth utilization for the past N seconds* and then average it out in each display loop iteration.

Note that raw_mode should still work the same (displaying just what happened in the past 1 second).

*I wouldn't want N to be configurable, but I'm not sure what it should be - my gut says 10 but I think it should be tested to see what looks best

@imsnif imsnif added this to the Version 1.0.0 milestone Nov 30, 2019
@bvergnaud

This comment has been minimized.

Copy link

@bvergnaud bvergnaud commented Dec 30, 2019

I'm afraid I won't be much help with the implementation as I've no rust knowledge at all.
However seeing this issue (which I agree with wholeheartedly), I think you ought to have some weighting mechanism in order for the more recent values to have more impact on the averaged out value. Otherwise, if you have an application that goes through drastic changes in bandwidth usage, that averaged value will lag behind for a few seconds at every change point.
The example I'm thinking of right away would be an app such as spotify. Downloads heavily for a few seconds, and then is quiet for the remainder of the song.

@imsnif

This comment has been minimized.

Copy link
Owner Author

@imsnif imsnif commented Dec 31, 2019

I like some of the ideas here, and I feel whoever wants to tackle this issue will have quite a fun time optimizing stuff to make this feel good. I'm personally looking forward to playing around with what comes up. :)

@chobeat

This comment has been minimized.

Copy link
Contributor

@chobeat chobeat commented Jan 3, 2020

can you point out where the refresh rate of 1 second is specified? Is it implicit somewhere because I couldn't find it.

@munntjlx

This comment has been minimized.

Copy link

@munntjlx munntjlx commented Jan 3, 2020

You might want to model this on something like iftop, which has a very nice display that does 'running' averages by top 10. They even have a nice logarythimic mode which makes it really easy to see who is being naughty.

@imsnif

This comment has been minimized.

Copy link
Owner Author

@imsnif imsnif commented Jan 4, 2020

So, right now what happens is quite hacky. The display thread updates the UI every second (minus the time it took it to render, so it's exact). This happens here: https://github.com/imsnif/bandwhich/blob/master/src/main.rs#L164

What I would do to start out with, is have some sort of fixed sized list of the last N seconds and display its average - removing old entries from the list to make room for new ones. If we have that infrastructure, we can easily have discussions about weighted averages, and the nice suggestions from @bvergnaud and @munntjlx. But ofc, if someone wants to take a different approach - I'd be interested in hearing more.

@chobeat

This comment has been minimized.

Copy link
Contributor

@chobeat chobeat commented Jan 5, 2020

I would go for the list and retaining all the information at this stage and eventually optimize it for memory usage with pre-computed averages after we consolidate the feature set.

@imsnif

This comment has been minimized.

Copy link
Owner Author

@imsnif imsnif commented Jan 5, 2020

I agree in principle @chobeat - just that I'd be sure to clear the list after a set amount, otherwise we'd just keep taking more and more memory :)

Would you like to work on this?

@chobeat

This comment has been minimized.

Copy link
Contributor

@chobeat chobeat commented Jan 5, 2020

I would, but I'm not sure I understand the codebase well enough to do it. I tried to think on how to do it but I'm lost. I think somebody more well-versed with Rust should do it.

@imsnif imsnif mentioned this issue Jan 8, 2020
@imsnif imsnif added the enhancement label Jan 11, 2020
@vlmutolo

This comment has been minimized.

Copy link

@vlmutolo vlmutolo commented Jan 14, 2020

@imsnif, you should also consider an exponential moving average. It has a few nice properties that are relevant here:

  1. The implementation is easy. Each process only needs to store a single number because the update function is iterative (new_smooth = alpha * new_raw + (1-alpha) * old_smooth). This way, you can avoid a ring buffer or some similar approach. The exponential weighted average, in a sense, contains the whole history of the time series in the latest number, but exponentially damped down so that old numbers are meaningless.
  2. It’s easily configurable with the parameter alpha, which is between 0 and 1. Closer to one means less smoothing.
  3. It’s a weighted average in the sense that more recent data are counted as more significant.

The downside of this approach is that it doesn’t support doing something like keeping track of a moving “total usage” number, which would probably require keeping a buffer.

@imsnif

This comment has been minimized.

Copy link
Owner Author

@imsnif imsnif commented Jan 14, 2020

This has been released in 0.9.0, yay! :D

@vlmutolo - for the moment we went for a normal run-of-the-mill average of the past 5 seconds. If you'd be interested in having a go at implementing your suggestion, I'd be very happy to review and merge it if it feels better.

@imsnif imsnif closed this Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.