Project

General

Profile

Actions

Bug #41667

closed

mgr/dashboard: Firefox ngx-datatable performance issue

Added by Patrick Seidensal over 4 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
Normal
Category:
UX
Target version:
% Done:

0%

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

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


Files

firefox-now.gif (195 KB) firefox-now.gif Patrick Seidensal, 09/05/2019 09:40 AM
firefox-after-line-removal.gif (379 KB) firefox-after-line-removal.gif Patrick Seidensal, 09/05/2019 09:40 AM
firefox-perf-fixed.png (138 KB) firefox-perf-fixed.png Patrick Seidensal, 09/05/2019 09:40 AM
firefox-perf-now.png (144 KB) firefox-perf-now.png Patrick Seidensal, 09/05/2019 09:40 AM
Actions #1

Updated by Patrick Seidensal over 4 years ago

  • Category set to 152
  • Affected Versions v15.0.0 added
Actions #2

Updated by Patrick Seidensal over 4 years ago

  • Description updated (diff)
Actions #3

Updated by Volker Theile over 4 years 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.

Actions #4

Updated by Patrick Seidensal over 4 years 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?

Actions #5

Updated by Patrick Seidensal over 4 years ago

  • Description updated (diff)
Actions #6

Updated by Patrick Seidensal over 4 years ago

  • Description updated (diff)
Actions #7

Updated by Laura Paduano over 4 years ago

  • Description updated (diff)
Actions #8

Updated by Patrick Seidensal over 4 years ago

  • Pull request ID set to 30316
Actions #9

Updated by Patrick Seidensal over 4 years ago

  • Status changed from Need More Info to Fix Under Review
Actions #10

Updated by Lenz Grimmer over 4 years ago

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

Updated by Ernesto Puerta about 3 years ago

  • Project changed from mgr to Dashboard
  • Category changed from 152 to UX
Actions

Also available in: Atom PDF