* [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime
@ 2025-11-18 14:48 Dominik Csapak
2025-11-18 14:48 ` [pve-devel] [PATCH manager 2/2] ui: resource tree: only fire 'refresh' event when something changed Dominik Csapak
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Dominik Csapak @ 2025-11-18 14:48 UTC (permalink / raw)
To: pve-devel
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% .
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
www/manager6/tree/ResourceTree.js | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
index bb016f8c..4c4e2908 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);
@@ -186,7 +186,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)));
}
@@ -310,8 +310,6 @@ Ext.define('PVE.tree.ResourceTree', {
let stateid = 'rid';
const changedFields = [
- 'disk',
- 'maxdisk',
'vmid',
'name',
'type',
@@ -409,14 +407,25 @@ Ext.define('PVE.tree.ResourceTree', {
}
}
- // tree item has been updated
- for (const field of changedFields) {
- if (item.data[field] !== olditem.data[field]) {
+ let diskuse = item.data.diskuse;
+ let oldDiskuse = olditem.data.diskuse;
+
+ if (diskuse !== undefined || oldDiskuse !== undefined) {
+ if (Math.abs(diskuse - oldDiskuse) > 0.009) {
changed = true;
- break;
}
}
- // FIXME: also test filterfn()?
+
+ 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()?
+ }
}
if (changed) {
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] [PATCH manager 2/2] ui: resource tree: only fire 'refresh' event when something changed
2025-11-18 14:48 [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime Dominik Csapak
@ 2025-11-18 14:48 ` Dominik Csapak
2025-12-11 13:30 ` [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime Fiona Ebner
2025-12-12 7:51 ` [pve-devel] superseded: " Dominik Csapak
2 siblings, 0 replies; 6+ messages in thread
From: Dominik Csapak @ 2025-11-18 14:48 UTC (permalink / raw)
To: pve-devel
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>
---
while this can be reduce the dom manipulation by quite a bit, it's a
change we should test very carefully, since it might be the case that i
overlooked something in the code that would need a refresh too.
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 4c4e2908..b8238c4f 100644
--- a/www/manager6/tree/ResourceTree.js
+++ b/www/manager6/tree/ResourceTree.js
@@ -354,6 +354,7 @@ Ext.define('PVE.tree.ResourceTree', {
let updateTree = function () {
store.suspendEvents();
+ let any_changed = false;
let rootnode;
if (firstUpdate) {
rootnode = Ext.create('PVETree', {
@@ -437,6 +438,7 @@ Ext.define('PVE.tree.ResourceTree', {
}
me.setIconCls(info);
olditem.commit();
+ any_changed = true;
}
if ((!item || moved) && olditem.isLeaf()) {
delete index[key];
@@ -454,6 +456,8 @@ Ext.define('PVE.tree.ResourceTree', {
let grandParent = parentNode.parentNode;
grandParent.removeChild(parentNode, true);
}
+
+ any_changed = true;
}
}
@@ -473,10 +477,13 @@ Ext.define('PVE.tree.ResourceTree', {
if (child) {
index[item.data.id] = child;
}
+ any_changed = true;
});
store.resumeEvents();
- store.fireEvent('refresh', store);
+ if (any_changed) {
+ store.fireEvent('refresh', store);
+ }
let foundChild = findNode(rootnode, lastsel?.data.id);
@@ -632,6 +639,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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime
2025-11-18 14:48 [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime Dominik Csapak
2025-11-18 14:48 ` [pve-devel] [PATCH manager 2/2] ui: resource tree: only fire 'refresh' event when something changed Dominik Csapak
@ 2025-12-11 13:30 ` Fiona Ebner
2025-12-11 13:32 ` Fiona Ebner
2025-12-11 13:57 ` Dominik Csapak
2025-12-12 7:51 ` [pve-devel] superseded: " Dominik Csapak
2 siblings, 2 replies; 6+ messages in thread
From: Fiona Ebner @ 2025-12-11 13:30 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Am 18.11.25 um 3:48 PM schrieb 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% .
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> www/manager6/tree/ResourceTree.js | 27 ++++++++++++++++++---------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
> index bb016f8c..4c4e2908 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);
> @@ -186,7 +186,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)));
> }
> @@ -310,8 +310,6 @@ Ext.define('PVE.tree.ResourceTree', {
> let stateid = 'rid';
>
> const changedFields = [
> - 'disk',
> - 'maxdisk',
> 'vmid',
> 'name',
> 'type',
> @@ -409,14 +407,25 @@ Ext.define('PVE.tree.ResourceTree', {
> }
> }
>
> - // tree item has been updated
> - for (const field of changedFields) {
> - if (item.data[field] !== olditem.data[field]) {
> + let diskuse = item.data.diskuse;
> + let oldDiskuse = olditem.data.diskuse;
> +
> + if (diskuse !== undefined || oldDiskuse !== undefined) {
> + if (Math.abs(diskuse - oldDiskuse) > 0.009) {
Is there a specific reason for using > 0.009? You write "resolution of
0.01", so intuitively, I'd expect >= 0.01 or, maybe if rounding matters,
>= 0.0095.
If we get a lot of incremental small changes after each other, we might
not detect a change even though it could've changed a lot overall?
> changed = true;
> - break;
> }
> }
> - // FIXME: also test filterfn()?
> +
> + 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()?
> + }
> }
>
> if (changed) {
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime
2025-12-11 13:30 ` [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime Fiona Ebner
@ 2025-12-11 13:32 ` Fiona Ebner
2025-12-11 13:57 ` Dominik Csapak
1 sibling, 0 replies; 6+ messages in thread
From: Fiona Ebner @ 2025-12-11 13:32 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Am 11.12.25 um 2:31 PM schrieb Fiona Ebner:
> Is there a specific reason for using > 0.009? You write "resolution of
> 0.01", so intuitively, I'd expect >= 0.01 or, maybe if rounding
> matters, >= 0.0095.
There was accidental quoting because of '>' at the beginning of the line
here.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime
2025-12-11 13:30 ` [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime Fiona Ebner
2025-12-11 13:32 ` Fiona Ebner
@ 2025-12-11 13:57 ` Dominik Csapak
1 sibling, 0 replies; 6+ messages in thread
From: Dominik Csapak @ 2025-12-11 13:57 UTC (permalink / raw)
To: Fiona Ebner, Proxmox VE development discussion
On 12/11/25 2:30 PM, Fiona Ebner wrote:
> Am 18.11.25 um 3:48 PM schrieb 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% .
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> www/manager6/tree/ResourceTree.js | 27 ++++++++++++++++++---------
>> 1 file changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
>> index bb016f8c..4c4e2908 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);
>> @@ -186,7 +186,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)));
>> }
>> @@ -310,8 +310,6 @@ Ext.define('PVE.tree.ResourceTree', {
>> let stateid = 'rid';
>>
>> const changedFields = [
>> - 'disk',
>> - 'maxdisk',
>> 'vmid',
>> 'name',
>> 'type',
>> @@ -409,14 +407,25 @@ Ext.define('PVE.tree.ResourceTree', {
>> }
>> }
>>
>> - // tree item has been updated
>> - for (const field of changedFields) {
>> - if (item.data[field] !== olditem.data[field]) {
>> + let diskuse = item.data.diskuse;
>> + let oldDiskuse = olditem.data.diskuse;
>> +
>> + if (diskuse !== undefined || oldDiskuse !== undefined) {
>> + if (Math.abs(diskuse - oldDiskuse) > 0.009) {
>
> Is there a specific reason for using > 0.009? You write "resolution of
> 0.01", so intuitively, I'd expect >= 0.01 or, maybe if rounding matters,
>> = 0.0095.
>
yeah >= 0.01 is better than whatever i wrote here :P
> If we get a lot of incremental small changes after each other, we might
> not detect a change even though it could've changed a lot overall?
this is only true if another field that triggers a rerender would also
change, in which case we'd also re-render the storage usage anyway.
if the item does not change, we don't overwrite the old data
this part is outside the git context:
```
if (changed) {
olditem.beginEdit();
let info = olditem.data;
Ext.apply(info, item.data)
if (info.id !== oldid) {
info.id = oldid;
}
me.setIconCls(info);
olditem.commit();
}
```
so only if any of the relevant data changes we update
the element in the tree store
>
>> changed = true;
>> - break;
>> }
>> }
>> - // FIXME: also test filterfn()?
>> +
>> + 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()?
>> + }
>> }
>>
>> if (changed) {
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] superseded: [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime
2025-11-18 14:48 [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime Dominik Csapak
2025-11-18 14:48 ` [pve-devel] [PATCH manager 2/2] ui: resource tree: only fire 'refresh' event when something changed Dominik Csapak
2025-12-11 13:30 ` [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime Fiona Ebner
@ 2025-12-12 7:51 ` Dominik Csapak
2 siblings, 0 replies; 6+ messages in thread
From: Dominik Csapak @ 2025-12-12 7:51 UTC (permalink / raw)
To: pve-devel
superseded by v2:
https://lore.proxmox.com/pve-devel/20251212074419.275374-1-d.csapak@proxmox.com/
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-12-12 7:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-18 14:48 [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime Dominik Csapak
2025-11-18 14:48 ` [pve-devel] [PATCH manager 2/2] ui: resource tree: only fire 'refresh' event when something changed Dominik Csapak
2025-12-11 13:30 ` [pve-devel] [PATCH manager 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime Fiona Ebner
2025-12-11 13:32 ` Fiona Ebner
2025-12-11 13:57 ` Dominik Csapak
2025-12-12 7:51 ` [pve-devel] superseded: " Dominik Csapak
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.