Scale network graph based on time interval

RandyMcMillan commented at 6:46 am on January 27, 2022: contributor

The Network Graph is updated to scale based on the time interval selected by the QSlider. The graph now displays a sample history that is retained when the QSlider value is changed. Fixes issue #532

RandyMcMillan commented at 6:49 am on January 27, 2022: contributor

This is a short screen cast of the functionality.

https://user-images.githubusercontent.com/152159/151306403-456dee98-ccfa-4db4-90d6-8687936e8180.mov

RandyMcMillan commented at 1:46 pm on January 27, 2022: contributor

Here it is retaining 5 1/2 hours of samples - still snappy.

https://user-images.githubusercontent.com/152159/151371047-ac788b14-cb5f-4bc1-b7be-ca662cdc1bdf.mov

RandyMcMillan commented at 2:10 pm on January 27, 2022: contributor

Original issue description #7481 (2016)

https://github.com/bitcoin/bitcoin/issues/7481

DrahtBot commented at 9:17 pm on January 27, 2022: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	jonatack, w0xlt

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#524 (Replace int with std::chrono in for the timer->setInterval() argument by shaavan)
#492 (Show ToolTip on Network Traffic graph by rebroad)
#484 (Don’t clear traffic graph when changing interval by rebroad)
#473 (Enable a non-linear network traffic option (click to toggle between linear and non-linear) by rebroad)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

RandyMcMillan commented at 7:17 am on January 28, 2022: contributor

Commit https://github.com/bitcoin-core/gui/pull/539/commits/42922129d1d8232fceff271c753e86d5ff555d27 takes the relevant samples from the global sample list to determine tmax - which is used to calculate the y axis'.

https://user-images.githubusercontent.com/152159/151503461-d9682cce-be68-4d12-b4f1-e563077c1338.mp4

RandyMcMillan force-pushed on Jan 28, 2022

RandyMcMillan commented at 7:05 pm on January 28, 2022: contributor

Implemented PR #473 Log Scale Toggle

https://user-images.githubusercontent.com/152159/151606196-3981d2df-e0eb-476b-85f6-8fd6c0c07054.mp4

rsafier commented at 0:03 am on January 29, 2022: none

tested it out and seemed to behave as expected.

jonatack commented at 0:23 am on January 29, 2022: contributor

Concept ACK, thanks for tackling this. Will review/test shortly. Edit: initial testing looks good.

RandyMcMillan commented at 9:39 am on January 29, 2022: contributor

commit 83e521c - attaches the scale toggle to a mouse double click event opposed to a single click event. Toggling the scale is more intentional this way - plus it makes a single click event available for something more useful.

RandyMcMillan commented at 1:24 pm on January 29, 2022: contributor

In some of the previous screen shots - the colors look muddy - commit 8eeb1f1 adjusts the visual output to avoid the way the colors were mixing and creating a muddy effect.

https://user-images.githubusercontent.com/152159/151662598-84a5b3ef-8f0b-4c40-91c4-95e3b3012e4e.mp4

RandyMcMillan commented at 7:12 pm on January 29, 2022: contributor

I have more ideas related to the network graph - I will post a different PR based on this PR - with some other ideas…

Screen Shot 2022-01-29 at 2 10 01 PM (2)

w0xlt commented at 3:23 pm on January 30, 2022: contributor

Concept ACK

in src/qt/trafficgraphwidget.cpp:88 in 0902db8f31 outdated

87     painter.fillRect(rect(), Qt::black);
88 
89     if(fMax <= 0.0f) return;
90 
91-    QColor axisCol(Qt::gray);
92+    QColor axisCol(QColor(78,78,78,255));

rebroad commented at 9:32 am on February 1, 2022:

?

RandyMcMillan commented at 4:40 pm on February 1, 2022:

I didn’t check to see if there was a Qt defined dark gray - It is ideal to let the samples stand out above the scale lines - we can tweak this. Note: there is plenty of room unused in the side panel - I have some code laying around to define user preferred color palettes - could be a useful feature for usability/accessibility.

RandyMcMillan commented at 4:44 pm on February 1, 2022:

A thought was to implement Solarized Light/Dark color palettes as user options - I am a fan - and plenty of research has gone into these color schemes.

gui: network graph - remove uneccesary clear() when changing interval 9c9ea4460c

gui: network graph - scale view based on time interval 0fc40e3223

gui: network graph - retain enough samples for large time intervals 55bae833d0

gui: network graph - fix left margin to hide paths 1d8a548476

gui: network graph - use samples subset to determine tmax aaf1bc3dae

qt:refactor: Use std::chrono in TrafficGraphWidget class 85b6adb008

gui: network graph - add log-scale toggle c7ac1b824b

gui: network graph - add log-scale toggle on double click f0871d15d0

gui: network graph - clean up colors - look better in log scale 909494147e

gui: network graph - set initial QSlider value to first position (5 mins) 255774f35f

gui: netowrk graph - reduce DESIRED_SAMPLES to 300 bb5b4cc41c

gui: netowrk graph - linear scale default c0f46d293a

gui: network graph - show/hide panels based on window width/height

The debugwindow - network graph hides ui elements
btnClearTrafficGraph, lblGraphRange, sldGraphRange
when at minimumHeight, it also hides the groupBox
at minimumWidth - this maximizes the dimensions of
the traffic graph in these small dimensions.

874b2d8ced

in src/qt/trafficgraphwidget.cpp:164 in 0902db8f31 outdated

160@@ -135,32 +161,35 @@ void TrafficGraphWidget::updateRates()
161     nLastBytesIn = bytesIn;
162     nLastBytesOut = bytesOut;
163 
164-    while(vSamplesIn.size() > DESIRED_SAMPLES) {
165+    while(vSamplesIn.size()  > FLT_MAX - 1) {

rebroad commented at 9:33 am on February 1, 2022:

how much memory does this need?

concept ACK implementation NACK - it’s unnecessary to store so many sample points, given the granulatory can be decreased as the times period gets longer. Resampling as the data gets older is a better way (as it avoids wasting memory on data that will never be displayed).

RandyMcMillan commented at 3:57 pm on February 1, 2022:

I agree - I have been thinking about how to do this. Maybe pruning the list at larger intervals - keeping samples that are above a moving average. Open to suggestions on implementation. :)

Thanks for looking at this!

RandyMcMillan commented at 4:00 pm on February 1, 2022:

We can chunk the end of the list at DESIRED_SAMPLES intervals - calculate the moving average - pruning samples that fall below it.

RandyMcMillan force-pushed on Feb 2, 2022

RandyMcMillan commented at 9:32 pm on February 2, 2022: contributor

rebase to fix Centos CI issue 874b2d8 - add hide panel logic which collapses window to a “desktop widget” bb5b4cc - changed DESIRED_SAMPLES to 300 - reducing overall memory usage - going to try some other ideas to minimize memory usage. (interim adjustment based on @rebroad’s comment) c0f46d2 - the linear scale is the default presentation - this is familiar to users - a follow up PR should include a note on adding double click log scale to release notes.

commit 874b2d8 screen cast:

https://user-images.githubusercontent.com/152159/152240134-72f51e3b-697c-4065-a752-6108c71b8ac9.mp4

DrahtBot commented at 11:02 am on February 4, 2022: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

DrahtBot added the label Needs rebase on Feb 4, 2022

rebroad commented at 7:47 pm on February 7, 2022: contributor

I’ve been working on this, and I have a pull request almost ready to make - storing 600 samples per time range, and 14 ranges in total (from 5 minutes to 28 days), so 8400 samples in total.

https://user-images.githubusercontent.com/1530283/152994292-53a9906c-3a1f-4b84-97ff-036ebb1c60ac.mp4

Or, smoother scaling of the horizontal also, or too gimicky? (below)

https://user-images.githubusercontent.com/1530283/154365362-cb1a688e-4844-4de4-a569-0a27bfab63b3.mp4

The plan is to add notches on the slider for these main samples, but then allow the slider to stop in a few places in between, e.g. for 15m, 45m or 1h30m, 4h30m, 2days, etc.. the resolution will be slightly lower (as it’s not sampled at these ranges), but it’s probably nice to have these extra options.

RandyMcMillan commented at 8:07 pm on February 7, 2022: contributor

@rebroad - awesome!

hebasto added the label UX on Feb 9, 2022

hebasto renamed this:
~~gui: scale network graph based on time interval~~
Scale network graph based on time interval
on Feb 9, 2022

RandyMcMillan marked this as a draft on Feb 17, 2022

RandyMcMillan commented at 3:52 am on April 4, 2022: contributor

@rebroad - I am curious if your memory managment is ready…

I am happy to merge it into this PR…

hebasto commented at 1:19 pm on July 19, 2022: member

What is the status of this PR?

DrahtBot commented at 10:30 am on October 31, 2022: contributor

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

rebroad commented at 8:57 pm on December 10, 2022: contributor

What is the status of this PR?

I’m currently rebasing it. Should update soon.

DrahtBot commented at 1:02 am on March 10, 2023: contributor

There hasn’t been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

hebasto added the label Up for grabs on Mar 10, 2023

hebasto commented at 11:36 am on March 10, 2023: member

How do I label it as up for grabs?

Done.

fanquake closed this on Mar 10, 2023

bitcoin-core locked this on Mar 9, 2024

Scale network graph based on time interval #539

Reviews

Conflicts