[pve-devel] [PATCH manager v3 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime

[pve-devel] [PATCH manager v3 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime

[Dominik Csapak]

the resource store has a field 'diskuse' which it calculates on update.
Use that instead of calculating the value ourselves everytime.

For change detection, we only need a resolution of 0.01 (since we want
to use the percentage as integer) so check that the difference of old and new
is bigger than 0.9% .

This works because we only overwrite the values in the treestore if
anything changed, so multiple small changes to the diskuse will not be
lost.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
chnages from v2:
* use better names for variables
* check explicitely if both old and new or just one is defined
  and mark it as changed accordingly

 www/manager6/tree/ResourceTree.js | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
index 6ea28919..bdb0d6f8 100644
--- a/www/manager6/tree/ResourceTree.js
+++ b/www/manager6/tree/ResourceTree.js
@@ -57,7 +57,7 @@ Ext.define('PVE.tree.ResourceTree', {
                 let text = info.text;
                 let status = '';
                 if (info.type === 'storage') {
-                    let usage = info.disk / info.maxdisk;
+                    let usage = info.diskuse;
                     if (usage >= 0.0 && usage <= 1.0) {
                         let barHeight = (usage * 100).toFixed(0);
                         let remainingHeight = (100 - barHeight).toFixed(0);
@@ -190,7 +190,7 @@ Ext.define('PVE.tree.ResourceTree', {
             qtips.push(Ext.String.format(gettext('HA State: {0}'), info.hastate));
         }
         if (info.type === 'storage') {
-            let usage = info.disk / info.maxdisk;
+            let usage = info.diskuse;
             if (usage >= 0.0 && usage <= 1.0) {
                 qtips.push(Ext.String.format(gettext('Usage: {0}%'), (usage * 100).toFixed(2)));
             }
@@ -331,8 +331,6 @@ Ext.define('PVE.tree.ResourceTree', {
         let stateid = 'rid';
 
         const changedFields = [
-            'disk',
-            'maxdisk',
             'vmid',
             'name',
             'type',
@@ -430,14 +428,29 @@ Ext.define('PVE.tree.ResourceTree', {
                         }
                     }
 
-                    // tree item has been updated
-                    for (const field of changedFields) {
-                        if (item.data[field] !== olditem.data[field]) {
+                    let diskUsage = item.data.diskuse;
+                    let oldDiskUsage = olditem.data.diskuse;
+
+                    if (diskUsage !== undefined && oldDiskUsage !== undefined) {
+                        // we have an old and new value, so check if it changed enough
+                        if (Math.abs(diskUsage - oldDiskUsage) >= 0.01) {
                             changed = true;
-                            break;
                         }
+                    } else if (diskUsage !== undefined || oldDiskUsage !== undefined) {
+                        // we have either a new or an old value, but not both, always treat that as a change
+                        changed = true;
+                    }
+
+                    if (!changed) {
+                        // tree item has been updated
+                        for (const field of changedFields) {
+                            if (item.data[field] !== olditem.data[field]) {
+                                changed = true;
+                                break;
+                            }
+                        }
+                        // FIXME: also test filterfn()?
                     }
-                    // FIXME: also test filterfn()?
                 }
 
                 if (changed) {
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v3 2/2] ui: resource tree: only fire 'refresh' event when something changed

[Dominik Csapak]

to optimize the rendering to the dom, only fire the 'refresh' event of
the store at the end of the updateTree method when either:

* an element changed it's relevant data
* something moved
* a new element was inserted
* an element was removed

We also need to refresh the store when the UI options are reloaded, so
the tags get the correct color.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v2:
* adhere to camelCase casing style

 www/manager6/tree/ResourceTree.js | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
index bdb0d6f8..8bf1f5dd 100644
--- a/www/manager6/tree/ResourceTree.js
+++ b/www/manager6/tree/ResourceTree.js
@@ -375,6 +375,7 @@ Ext.define('PVE.tree.ResourceTree', {
         let updateTree = function () {
             store.suspendEvents();
 
+            let anyChanged = false;
             let rootnode;
             if (firstUpdate) {
                 rootnode = Ext.create('PVETree', {
@@ -462,6 +463,7 @@ Ext.define('PVE.tree.ResourceTree', {
                     }
                     me.setIconCls(info);
                     olditem.commit();
+                    anyChanged = true;
                 }
                 if ((!item || moved) && olditem.isLeaf()) {
                     delete index[key];
@@ -479,6 +481,8 @@ Ext.define('PVE.tree.ResourceTree', {
                         let grandParent = parentNode.parentNode;
                         grandParent.removeChild(parentNode, true);
                     }
+
+                    anyChanged = true;
                 }
             }
 
@@ -498,10 +502,13 @@ Ext.define('PVE.tree.ResourceTree', {
                 if (child) {
                     index[item.data.id] = child;
                 }
+                anyChanged = true;
             });
 
             store.resumeEvents();
-            store.fireEvent('refresh', store);
+            if (anyChanged) {
+                store.fireEvent('refresh', store);
+            }
 
             let foundChild = findNode(rootnode, lastsel?.data.id);
 
@@ -660,6 +667,9 @@ Ext.define('PVE.tree.ResourceTree', {
                     return true;
                 },
             });
+
+            // rerender
+            me.store.fireEvent('refresh', store);
         });
     },
 });
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager v3 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime

[Dominik Csapak]

ping

Re: [pve-devel] [PATCH manager v3 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime

[Jonas Theisen]

On 1/23/26 09:12, Dominik Csapak wrote:
> the resource store has a field 'diskuse' which it calculates on update.
> Use that instead of calculating the value ourselves everytime.
>
> For change detection, we only need a resolution of 0.01 (since we want
> to use the percentage as integer) so check that the difference of old and new
> is bigger than 0.9% .
>
> This works because we only overwrite the values in the treestore if
> anything changed, so multiple small changes to the diskuse will not be
> lost.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> <snip>

Applied and tested the patch on top of current pve-manager 9.2.18
Tested VM state changes and storage usage changes on a single node

Problem: The tooltip style usage shown when hovering over a storage
reports the usage with two decimal places so per mille.
Due to the update logic only triggering on changes of 1% or greater,
a change smaller than that will cause the tooltip to get out of sync
from the actual usage reported by the status page.

--
Tested-by: Jonas Theisen <j.theisen@proxmox.com>

Re: [pve-devel] [PATCH manager v3 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime

[Dominik Csapak]

On 9/9/26 11:22 AM, Jonas Theisen wrote:
> On 1/23/26 09:12, Dominik Csapak wrote:
>> the resource store has a field 'diskuse' which it calculates on update.
>> Use that instead of calculating the value ourselves everytime.
>>
>> For change detection, we only need a resolution of 0.01 (since we want
>> to use the percentage as integer) so check that the difference of old 
>> and new
>> is bigger than 0.9% .
>>
>> This works because we only overwrite the values in the treestore if
>> anything changed, so multiple small changes to the diskuse will not be
>> lost.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> <snip>
> 
> Applied and tested the patch on top of current pve-manager 9.2.18
> Tested VM state changes and storage usage changes on a single node
> 
> Problem: The tooltip style usage shown when hovering over a storage
> reports the usage with two decimal places so per mille.
> Due to the update logic only triggering on changes of 1% or greater,
> a change smaller than that will cause the tooltip to get out of sync
> from the actual usage reported by the status page.
> 

thanks for checking!

Ok so I think lowering the threshold to 0.01% difference probably
does not have the effect I'd hoped for (i.e. not refreshing too often)

So I'd either render the tooltip as whole percentage too, or
use 0.1% in both cases as a middle ground.

What do you think?

> -- 
> Tested-by: Jonas Theisen <j.theisen@proxmox.com>

Re: [pve-devel] [PATCH manager v3 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime

[Jonas Theisen]

On 9/9/26 11:29, Dominik Csapak wrote:
> On 9/9/26 11:22 AM, Jonas Theisen wrote:
>> On 1/23/26 09:12, Dominik Csapak wrote:
>>> the resource store has a field 'diskuse' which it calculates on update.
>>> Use that instead of calculating the value ourselves everytime.
>>>
>>> For change detection, we only need a resolution of 0.01 (since we want
>>> to use the percentage as integer) so check that the difference of 
>>> old and new
>>> is bigger than 0.9% .
>>>
>>> This works because we only overwrite the values in the treestore if
>>> anything changed, so multiple small changes to the diskuse will not be
>>> lost.
>>>
>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>>> ---
>>> <snip>
>>
>> Applied and tested the patch on top of current pve-manager 9.2.18
>> Tested VM state changes and storage usage changes on a single node
>>
>> Problem: The tooltip style usage shown when hovering over a storage
>> reports the usage with two decimal places so per mille.
>> Due to the update logic only triggering on changes of 1% or greater,
>> a change smaller than that will cause the tooltip to get out of sync
>> from the actual usage reported by the status page.
>>
>
> thanks for checking!
>
> Ok so I think lowering the threshold to 0.01% difference probably
> does not have the effect I'd hoped for (i.e. not refreshing too often)
>
> So I'd either render the tooltip as whole percentage too, or
> use 0.1% in both cases as a middle ground.
>
> What do you think?

My personal preference would be to render the tooltip as
rounded percentage since in my head it is just a quick view option.
If the user wants the exact number, they can look into the status page.
>
>> -- 
>> Tested-by: Jonas Theisen <j.theisen@proxmox.com>
>