Memory leak fixes for several views #7057
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ks memory - must test on firefox to see if this is a Chrome leak.
… One type of overlay plot (Rover Yaw) is still leaking.
…penmct into vue3-createApp-leak-fixes
…eateApp-leak-fixes
shefalijoshi
commented
Sep 14, 2023
ozyx
reviewed
Sep 14, 2023
ozyx
suggested changes
Sep 14, 2023
Codecov Report
@@ Coverage Diff @@
## master #7057 +/- ##
==========================================
+ Coverage 55.12% 55.25% +0.12%
==========================================
Files 650 649 -1
Lines 25972 26041 +69
Branches 2539 2547 +8
==========================================
+ Hits 14318 14389 +71
- Misses 10953 10958 +5
+ Partials 701 694 -7
*This pull request uses carry forward flags. Click here to find out more.
... and 34 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Current Playwright Test Results Summary✅ 14 Passing Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 09/20/2023 05:32:51pm UTC) Run DetailsRunning Workflow e2e-couchdb on Github Actions Commit: d25be18 Started: 09/20/2023 05:31:02pm UTC Current Playwright Test Results Summary✅ 139 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 09/20/2023 05:32:51pm UTC)
|
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Time List Create a Time List, add a single Plan to it and verify all the activities are displayed with no milliseconds
Retry 1 • Initial Attempt |
1.54% (2)2 / 130 runsfailed over last 7 days |
50% (65)65 / 130 runsflaked over last 7 days |
📄 functional/plugins/notebook/notebookSnapshots.e2e.spec.js • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Snapshot image tests Can drop an image onto a notebook and create a new entry
Retry 1 • Initial Attempt |
0% (0)0 / 41 runsfailed over last 7 days |
78.05% (32)32 / 41 runsflaked over last 7 days |
…penmct into vue3-createApp-leak-fixes
ozyx
reviewed
Sep 20, 2023
| @@ -56,7 +56,7 @@ export default { | |||
| mounted() { | |||
| this.openmct.editor.on('isEditing', this.setEditMode); | |||
| }, | |||
| beforeUnmounted() { | |||
…eateApp-leak-fixes
shefalijoshi
commented
Sep 20, 2023
| @@ -81,7 +81,7 @@ export default { | |||
| this.listenToConditionSetChanges(); | |||
| } | |||
| }, | |||
| beforeUnmount() { | |||
| unmounted() { | |||
There was a problem hiding this comment.
This works - I don't understand why.
akhenry
approved these changes
Sep 20, 2023
ozyx
added a commit
that referenced
this pull request
Sep 20, 2023
* Change the mount utility to use Vue's createApp and defineComponent methods * Fix display layout memory leaks caused by `getSelectionContext` * fix some display layout leaks due to use of slots * Fix imagery memory leak (removed span tag). NOTE: CompassRose svg leaks memory - must test on firefox to see if this is a Chrome leak. * Fix ActionsAPI action collection and applicable actions leak. * Fix flexible layout memory leaks - remove listeners on unmount. NOTE: One type of overlay plot (Rover Yaw) is still leaking. * pass in the el on mount * e2e test config and spec changes * Remove mounting of limit lines. Use components directly * test: remove `.only()` * Fix display layout memory leaks * Enable passing tests * e2e README and appActions should be what master has. * lint: add word to cspell list * lint: fixes * lint:fix * fix: revert `el` change * fix: remove empty span * fix: creating shapes in displayLayout * fix: avoid `splice` as it loses reactivity * test: reduce timeout time * quick fixes * add prod mode and convert the test config to select the correct mode * Fix webpack prod config * Add launch flag for exposing window.gc * never worked * explicit naming * rename * We don't need to destroy view providers * test: increase timeout time * test: unskip all mem tests * fix(vue-loader): disable static hoisting * chore: run `test:perf:memory` * Don't destroy view providers * Move context menu once listener to beforeUnmount instead. * Disconnect all resize observers on unmount * Delete Test vue component * Use beforeUnmount and remove splice(0) in favor of [] for emptying arrays * re-structure * fix: unregister listener in pane.vue * test: tweak timeouts * chore: lint:fix * test: unskip perf tests * fix: unregister events properly * fix: unregister listener * fix: unregister listener * fix: unregister listener * fix: use `unmounted()` * fix: unregister listeners * fix: unregister listener properly * chore: lint:fix * test: fix imagery layer toggle test * test: increase timeout * Don't use anonymous functions for listeners * Destroy objects and event listeners properly * Delete config stores that are created by components * Use the right unmount hook. Destroy mounted view on unmount. * Use unmounted, not beforeUnmounted * Lint fixes * Fix time strip memory leak * Undo unneeded change for memory leaks. * chore: combine common webpack configs --------- Co-authored-by: Jesse Mazzella <jesse.d.mazzella@nasa.gov> Co-authored-by: John Hill <john.c.hill@nasa.gov>
ozyx
added a commit
that referenced
this pull request
Sep 20, 2023
* Change the mount utility to use Vue's createApp and defineComponent methods * Fix display layout memory leaks caused by `getSelectionContext` * fix some display layout leaks due to use of slots * Fix imagery memory leak (removed span tag). NOTE: CompassRose svg leaks memory - must test on firefox to see if this is a Chrome leak. * Fix ActionsAPI action collection and applicable actions leak. * Fix flexible layout memory leaks - remove listeners on unmount. NOTE: One type of overlay plot (Rover Yaw) is still leaking. * pass in the el on mount * e2e test config and spec changes * Remove mounting of limit lines. Use components directly * test: remove `.only()` * Fix display layout memory leaks * Enable passing tests * e2e README and appActions should be what master has. * lint: add word to cspell list * lint: fixes * lint:fix * fix: revert `el` change * fix: remove empty span * fix: creating shapes in displayLayout * fix: avoid `splice` as it loses reactivity * test: reduce timeout time * quick fixes * add prod mode and convert the test config to select the correct mode * Fix webpack prod config * Add launch flag for exposing window.gc * never worked * explicit naming * rename * We don't need to destroy view providers * test: increase timeout time * test: unskip all mem tests * fix(vue-loader): disable static hoisting * chore: run `test:perf:memory` * Don't destroy view providers * Move context menu once listener to beforeUnmount instead. * Disconnect all resize observers on unmount * Delete Test vue component * Use beforeUnmount and remove splice(0) in favor of [] for emptying arrays * re-structure * fix: unregister listener in pane.vue * test: tweak timeouts * chore: lint:fix * test: unskip perf tests * fix: unregister events properly * fix: unregister listener * fix: unregister listener * fix: unregister listener * fix: use `unmounted()` * fix: unregister listeners * fix: unregister listener properly * chore: lint:fix * test: fix imagery layer toggle test * test: increase timeout * Don't use anonymous functions for listeners * Destroy objects and event listeners properly * Delete config stores that are created by components * Use the right unmount hook. Destroy mounted view on unmount. * Use unmounted, not beforeUnmounted * Lint fixes * Fix time strip memory leak * Undo unneeded change for memory leaks. * chore: combine common webpack configs --------- Co-authored-by: Shefali Joshi <simplyrender@gmail.com> Co-authored-by: John Hill <john.c.hill@nasa.gov>
17 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #4406 #4209
Describe your changes:
perftests<method>.bind(this...)or anonymous functions()=>{}.hoistStaticoption which was holding onto refsAll Submissions:
Author Checklist
Reviewer Checklist