]> git.proxmox.com Git - proxmox-widget-toolkit.git/commit
override deselection in CheckboxModel to improve performance
authorThomas Lamprecht <t.lamprecht@proxmox.com>
Mon, 22 Mar 2021 06:08:11 +0000 (07:08 +0100)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Mon, 22 Mar 2021 15:47:04 +0000 (16:47 +0100)
commit1f628e9ca6b5f49326539000166e34b2732c5555
treecfd6aa82cfdeb288dc28fb3023fda7ae52813a60
parentc2c12542d71edf01e80e0960e701eeef8e9bbd3b
override deselection in CheckboxModel to improve performance

After the UI hang for tens of seconds for a few thousand elements got
deselected I investigated with Firefox developer tools performance
analysis, where the waterfall view showed that most of the time was
spent in array splicing.

Previously, all removed elements got removed on by one from the
`selected` Ext.util.Collection - which is basically an helper class
around arrays and objects, most of it may have become obsolete with
modern browsers. The single remove resulted into further splicing of
the array, and all in all it resulted in a dramatically increased
complexity, ~ O(n^3).

The "remove one by one" logic comes highly probably from the fact
that users can register a `beforedeselection` listener which can
block a removal of a specific record. But, that's not used by us and
not really something one would often need in practice, but still its
a documented feature of ExtJS grids we want to keep; so go for an
alternative.
So, override `doDeselect` and change the old removal logic to one
that first record those entries which got blocked from removal and
remove them in one go from the "to-be-removed" collection.

Before/After this patch on my FF 86.0.1 with my i9-9900K and
deselecting ~10k records went from ~40s to about 33 ms total, so for
that case we went 1000x faster.

The remaining time is now mostly spend in the event fire/handling
logic, but even with 50k records we spent <<500ms in total, so not
too bad and thus kept as is for now.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Reviewed-By: Dominik Csapak <d.csapak@proxmox.com>
Tested-By: Stoiko Ivanov <s.ivanov@proxmox.com>
Reviewed-By: Stoiko Ivanov <s.ivanov@proxmox.com>
src/Toolkit.js