Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sorting the command history table causes Open MCT to freeze #384

Closed
3 of 5 tasks
akhenry opened this issue Oct 17, 2023 · 7 comments · Fixed by nasa/openmct#7154
Closed
3 of 5 tasks

Sorting the command history table causes Open MCT to freeze #384

akhenry opened this issue Oct 17, 2023 · 7 comments · Fixed by nasa/openmct#7154

Comments

@akhenry
Copy link
Owner

akhenry commented Oct 17, 2023

Summary

Sorting the command history table causes Open MCT.

Impact Check List

Causes the entire tab that the command history table is in to be unusable, and Chrome to be unresponsive.

  • Data loss or misrepresented data?
  • Regression? Did this used to work or has it always been broken?
  • Is there a workaround available?
  • Does this impact a critical component?
  • Is this just a visual bug with no functional impact?

Steps to Reproduce

  1. Open the command history table with some rows in it. (there were ~30 commands in history when this issue was observed)
  2. Click on the "Generation Time" column header 3-4 times
  3. Observe that the tab becomes unresponsive.

Environment

  • Open MCT Version: 3.0.2
  • Deployment Type: Prod
  • OS: Ubuntu 20.0.4
  • Browser: Chrome 115.0.5790.102
@akhenry akhenry added type:bug Something isn't working bug:regression labels Oct 17, 2023
@akhenry akhenry added this to the Target:3.1.2 milestone Oct 17, 2023
@scottbell scottbell self-assigned this Oct 17, 2023
@scottbell
Copy link
Collaborator

After fiddling with trying to recreate this, here's a couple of observations

  • Need to have a TelemetryTable with some data in it.
  • Clicking the column over and over slows, and then eventually freezes the browser
  • CPU spikes while this is happening
  • Appears to happen after having table open for awhile. Navigating away and back appears to resolve the issue
  • Looking at performance trace while this is happening, it looks like a lot of Vue updates are happening, so perhaps the table has too much reactive data (like the plots did)?

@scottbell
Copy link
Collaborator

scottbell commented Oct 18, 2023

FWIW, there's a memory leak. Here's the memlab report of a scenario where i click the the "Time" header 300 times in a fixed time setting with 18 days of sine wave generator data whose data rate is 0.0001 Hz:

$ memlab run --scenario scenario.js    
page-load[101.8MB](baseline)[s1] > action-on-page[102.5MB](target)[s2] > revert[102MB](final)[s3]  

total time: 12min
Memory usage across all steps:
117.9 ____________
103.3 ____________
 88.7    _   _   _
 74.0    _   _   _
 59.4    _   _   _
 44.8    _   _   _
 30.2    _   _   _
 15.6    _   _   _
  1.0    _   _   _
      1   2   3   

MemLab found 1 leak(s)

--Similar leaks in this run: 1158--
--Retained size of leaked objects: 95.5KB--
[<synthetic>] (synthetic) @1 [103.3MB]
  --2 (shortcut)--->  [Window / http://localhost:8080] (object) @6199 [33.1KB]
  --__VUE_INSTANCE_SETTERS__ (property)--->  [Array] (object) @181935 [164 bytes]
  --0 (element)--->  [<closure>] (closure) @182811 [72 bytes]
  --context (internal)--->  [<function scope>] (object) @179463 [235.2KB]
  --reactiveMap (variable)--->  [WeakMap] (object) @182307 [32.8KB]
  --table (internal)--->  [<array>] (array) @1972831 [32.7KB]
  --4282 / part of key (Object @850793) -> value (system / JSProxy @1425967) pair in WeakMap (table @1972831) (internal)--->  [system / JSProxy] (hidden) @1425967 [16 bytes]
  --1 (hidden)--->  [Object] (object) @850793 [852 bytes]
  --scrollable (property)--->  [Detached HTMLDivElement] (native) @95067 [452 bytes]
  --__vueParentComponent (property)--->  [Object] (object) @546897 [78.1KB]
  --subTree (property)--->  [Object] (object) @1878371 [73.1KB]
  --dynamicChildren (property)--->  [Array] (object) @1922375 [196 bytes]
  --10 (element)--->  [Object] (object) @1922403 [1.6KB]
  --children (property)--->  [Array] (object) @1922169 [424 bytes]
  --35 (element)--->  [Object] (object) @1801815 [1.2KB]
  --el (property)--->  [Detached HTMLTableRowElement] (native) @90965 [19.1KB]
  --7 (element)--->  [Detached Text] (native) @90967 [108 bytes]
  --4 (element)--->  [Detached HTMLTableCellElement] (native) @90971 [2.2KB]
  --11 (element)--->  [Detached HTMLTableCellElement] (native) @90981 [2.2KB]
  --7 (element)--->  [Detached Text] (native) @93386 [80 bytes]

though pretty small. I'll keep digging.

@scottbell
Copy link
Collaborator

scottbell commented Oct 19, 2023

Tried this morning automating some sorting of the Commands table, and even after 3000 clicks, I didn't see any slow down. However, if the system is really bogged down (e.g., lots of plots), the sort on a telemetry table with a lot of columns (like the Commands table) can cause the CPU to spike more:

Screen.Recording.2023-10-19.at.12.00.43.PM.mov

as each column is getting rerendered by Vue. I wonder if generationTime is particularly bad as we're basically needing to sort big integers?

@scottbell
Copy link
Collaborator

scottbell commented Oct 19, 2023

if we fall through the else here:

if (this.firstRowInSortOrder(lastIncomingRow, firstExistingRow) === lastIncomingRow) {
        this.insertOrUpdateRows(sortedRows, true);
      } else if (this.firstRowInSortOrder(lastExistingRow, firstIncomingRow) === lastExistingRow) {
        this.insertOrUpdateRows(sortedRows, false);
      } else {
        this.mergeSortedRows(sortedRows);
      }

namely, we’re in the middle of the row and hence we hit mergeSortedRows, then we fall through to:

mergeSortedRows(rows) {
      const mergedRows = [];
      let i = 0;
      let j = 0;

      while (i < this.rows.length && j < rows.length) {
        const existingRow = this.rows[i];
        const incomingRow = rows[j];

        const index = this.getInPlaceUpdateIndex(incomingRow);
        if (index > -1) {
          this.updateRowInPlace(incomingRow, index);
        } else {
          if (this.firstRowInSortOrder(existingRow, incomingRow) === existingRow) {
            mergedRows.push(existingRow);
            i++;
          } else {
            mergedRows.push(incomingRow);
            j++;
          }
        }
      }

has some bad logic here as if index > -1 , we never exit the loop. To fix:

if (index > -1) {
          this.updateRowInPlace(incomingRow, index);
          i++;
          j++;
        }

@scottbell
Copy link
Collaborator

scottbell commented Oct 19, 2023

To test:

  1. Create a command stack like so in YAMCS:
image
  1. Go to the command queues, and click the little hamburger to set the queue to "HOLD":
image
  1. Go back to the command stack and fire a bunch of commands, they should show up in the Open MCT Commands window.
  2. Sort the Commands Table in Open MCT by "Sequence Number".
  3. Go back to the command queue in YAMCS, and fire a command by clicking "Accept" from the middle of your stack. The browser tab will freeze. Or not if you've applied the fix!

@ozyx
Copy link
Collaborator

ozyx commented Oct 26, 2023

Verified Fixed -- Testathon 10/26/23

@charlesh88
Copy link

Verified fixed Testathon 2023-10-23.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants