This small change means that qx.html.Element.flush() is only called from qx.ui.basic.Image.__checkForContentElementReplacement when it is absolutely necessary to do so.
The result of which is a significant improvement in performance, enough to feel as a user in contexts where there are sufficient instances. In our case, this was a scrollable region containing about 140 qx.ui.basic.Images used to display fontAwesome icons, these are only loaded when visible within the container, which fits around 40 at a time.
The table below shows some figures pulled from Chrome's (Dev v122) performance profiler. The point of interest is the cost of setting the source property, for which the table shows the total time spent within the method (_applySource (ms)), and the percentage of the total time of the action which was spent within the method (_applySource (% total)). It also shows the total time of the action itself (total (ms)).
optimise
_applySource (ms)
_applySource (% total)
total (ms)
N
82.9
27.0
306.6
N
117.4
26.8
437.4
N
115.7
27.0
428.3
Y
3.7
1.2
316.4
Y
3.9
1.2
327.0
Y
4.2
1.3
331.1
This was performed several times for both the original code prior to this commit (shown as optimise=N) and for the code after this commit (optimise=Y). More tests with similar results were performed but not recorded in the table.
As the table above shows quite clearly, adding this small condition represents a fairly significant performance improvement.
The action which generated the performance profile was to first reload the page completely to ensure a fair starting point, then begin recording, scroll to the bottom with the mousewheel, then end the recording. The figures were taken by selecting the profiler task corresponding to the scroll, not the full time between start/end recording.
This has been tested against the original reproduction of #7484 (BZ#7447) and shows no sign of breaking.
Related: #7484 (BZ#7447) (see: Image.js#L712)
This small change means that
qx.html.Element.flush()is only called fromqx.ui.basic.Image.__checkForContentElementReplacementwhen it is absolutely necessary to do so.The result of which is a significant improvement in performance, enough to feel as a user in contexts where there are sufficient instances. In our case, this was a scrollable region containing about 140
qx.ui.basic.Images used to display fontAwesome icons, these are only loaded when visible within the container, which fits around 40 at a time.The table below shows some figures pulled from Chrome's (Dev v122) performance profiler. The point of interest is the cost of setting the
sourceproperty, for which the table shows the total time spent within the method (_applySource (ms)), and the percentage of the total time of the action which was spent within the method (_applySource (% total)). It also shows the total time of the action itself (total (ms)).This was performed several times for both the original code prior to this commit (shown as optimise=N) and for the code after this commit (optimise=Y). More tests with similar results were performed but not recorded in the table.
As the table above shows quite clearly, adding this small condition represents a fairly significant performance improvement.
The action which generated the performance profile was to first reload the page completely to ensure a fair starting point, then begin recording, scroll to the bottom with the mousewheel, then end the recording. The figures were taken by selecting the profiler task corresponding to the scroll, not the full time between start/end recording.
This has been tested against the original reproduction of #7484 (BZ#7447) and shows no sign of breaking.