Project

General

Profile

Bug #41667

mgr/dashboard: Firefox ngx-datatable performance issue

Added by Patrick Seidensal 6 months ago. Updated 5 months ago.

Status:
Resolved
Priority:
Normal
Category:
dashboard/usability
Target version:
% Done:

0%

Source:
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature:

Description

The data table (ngx-datatable) used in the dashboard is very slow in Firefox. Hovering columns takes some time (<1s) to highlight the row that's hovered. When trying to navigate such a table using the page navigation, I need to wait some time (<1s) to be able to click the button for the next page. Although the browser has the data, loading a 10 rows page takes more than a second. Summarized, the performance and hence, the user experience with Firefox is pretty bad.

This problem is not noticable using Chrome.

Before fix:

Performance graphs:

After fix (one line removal):

Performance graphs:

The changes:

Index: src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.ts    (date 1567608360000)
+++ src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.ts    (date 1567676724112)
@@ -179,7 +179,6 @@
   ngOnInit() {
     // ngx-datatable triggers calculations each time mouse enters a row,
     // this will prevent that.
-    window.addEventListener('mouseenter', (event) => event.stopPropagation(), true);
     this._addTemplates();
     if (!this.sorts) {
       // Check whether the specified identifier exists.

https://github.com/ceph/ceph/blob/master/src/pybind/mgr/dashboard/frontend/src/app/shared/datatable/table/table.component.ts#L182

As the fix is basically the removal of another fix, I'd like to discuss the issue here first.

Relates to

- https://github.com/swimlane/ngx-datatable/issues/1513
- https://github.com/ceph/ceph/pull/28118
- https://github.com/swimlane/ngx-datatable/issues/1513

firefox-now.gif View (195 KB) Patrick Seidensal, 09/05/2019 09:40 AM

firefox-after-line-removal.gif View (379 KB) Patrick Seidensal, 09/05/2019 09:40 AM

firefox-perf-fixed.png View (138 KB) Patrick Seidensal, 09/05/2019 09:40 AM

firefox-perf-now.png View (144 KB) Patrick Seidensal, 09/05/2019 09:40 AM

History

#1 Updated by Patrick Seidensal 6 months ago

  • Category set to dashboard/usability
  • Affected Versions v15.0.0 added

#2 Updated by Patrick Seidensal 6 months ago

  • Description updated (diff)

#3 Updated by Volker Theile 6 months ago

Did you realize the comment above the removed line? The code has been added by intention. I assume the removal will result in unexpected behaviour. You should make sure this will not the case.

#4 Updated by Patrick Seidensal 6 months ago

Volker Theile wrote:

Did you realize the comment above the removed line? The code has been added by intention. I assume the removal will result in unexpected behaviour. You should make sure this will not the case.

Yes, this is a discussion about the implications of that line, as states in the description of this issue:

As the fix is basically the removal of another fix, I'd like to discuss the issue here first.

Personally, I was not yet able to reproduce an issue by removing that line. But, on the other hand, I identified a reproducible issue when this line is included in the code. Hence my question is: How is the issue fixed by that line reproducible and can it probably be fixed by doing something else or, if it can't be fixed by something else, is it it worth having such a big performance impact on Firefox users?

#5 Updated by Patrick Seidensal 6 months ago

  • Description updated (diff)

#6 Updated by Patrick Seidensal 6 months ago

  • Description updated (diff)

#7 Updated by Laura Paduano 6 months ago

  • Description updated (diff)

#8 Updated by Patrick Seidensal 6 months ago

  • Pull request ID set to 30316

#9 Updated by Patrick Seidensal 5 months ago

  • Status changed from Need More Info to Fix Under Review

#10 Updated by Lenz Grimmer 5 months ago

  • Status changed from Fix Under Review to Resolved
  • Target version set to v15.0.0

Also available in: Atom PDF