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

Memory leak fixes for several views #7057

Merged
merged 69 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from 63 commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
76c6d32
Change the mount utility to use Vue's createApp and defineComponent m…
shefalijoshi Aug 25, 2023
87f5298
Merge branch 'master' of https://github.com/nasa/openmct into vue3-us…
shefalijoshi Sep 11, 2023
3135f7c
Fix display layout memory leaks caused by `getSelectionContext`
shefalijoshi Sep 12, 2023
d088fc8
fix some display layout leaks due to use of slots
shefalijoshi Sep 13, 2023
438af25
Fix imagery memory leak (removed span tag). NOTE: CompassRose svg lea…
shefalijoshi Sep 13, 2023
584d9a6
Fix ActionsAPI action collection and applicable actions leak.
shefalijoshi Sep 13, 2023
cb1412b
Fix flexible layout memory leaks - remove listeners on unmount. NOTE:…
shefalijoshi Sep 13, 2023
faec175
pass in the el on mount
shefalijoshi Sep 13, 2023
87ee90f
e2e test config and spec changes
shefalijoshi Sep 13, 2023
6587c98
Remove mounting of limit lines. Use components directly
shefalijoshi Sep 13, 2023
b61984d
test: remove `.only()`
ozyx Sep 13, 2023
50ddaa1
Fix display layout memory leaks
shefalijoshi Sep 14, 2023
324c270
Enable passing tests
shefalijoshi Sep 14, 2023
dad610f
Merge branch 'vue3-createApp-leak-fixes' of https://github.com/nasa/o…
shefalijoshi Sep 14, 2023
59fc686
Merge branch 'master' of https://github.com/nasa/openmct into vue3-cr…
shefalijoshi Sep 14, 2023
211fcaa
e2e README and appActions should be what master has.
shefalijoshi Sep 14, 2023
e7ed49d
Merge branch 'master' into vue3-createApp-leak-fixes
ozyx Sep 14, 2023
ce026b0
lint: add word to cspell list
ozyx Sep 14, 2023
c4712c1
lint: fixes
ozyx Sep 14, 2023
7a99a3e
lint:fix
ozyx Sep 14, 2023
7d29d10
fix: revert `el` change
ozyx Sep 14, 2023
0d8f7d1
fix: remove empty span
ozyx Sep 14, 2023
b368c93
fix: creating shapes in displayLayout
ozyx Sep 14, 2023
0830236
fix: avoid `splice` as it loses reactivity
ozyx Sep 14, 2023
e326a19
test: reduce timeout time
ozyx Sep 14, 2023
fb7c31e
quick fixes
unlikelyzero Sep 14, 2023
442def3
add prod mode and convert the test config to select the correct mode
unlikelyzero Sep 15, 2023
ca19785
Fix webpack prod config
shefalijoshi Sep 15, 2023
d072b4a
Add launch flag for exposing window.gc
shefalijoshi Sep 15, 2023
b1fe579
never worked
unlikelyzero Sep 15, 2023
7efc49a
explicit naming
unlikelyzero Sep 15, 2023
fd04f4c
rename
unlikelyzero Sep 15, 2023
47e3489
We don't need to destroy view providers
shefalijoshi Sep 15, 2023
19f3785
Merge branch 'vue3-createApp-leak-fixes' of https://github.com/nasa/o…
shefalijoshi Sep 15, 2023
11c328d
test: increase timeout time
ozyx Sep 16, 2023
17dd486
test: unskip all mem tests
ozyx Sep 16, 2023
df08173
fix(vue-loader): disable static hoisting
ozyx Sep 16, 2023
f8b8b4e
chore: run `test:perf:memory`
ozyx Sep 16, 2023
d0e2325
Don't destroy view providers
shefalijoshi Sep 17, 2023
dafb31e
Move context menu once listener to beforeUnmount instead.
shefalijoshi Sep 17, 2023
db8a536
Disconnect all resize observers on unmount
shefalijoshi Sep 17, 2023
33ce725
Delete Test vue component
shefalijoshi Sep 17, 2023
2be40c4
Use beforeUnmount and remove splice(0) in favor of [] for emptying ar…
shefalijoshi Sep 17, 2023
fb15df6
re-structure
unlikelyzero Sep 18, 2023
24f000a
fix: unregister listener in pane.vue
ozyx Sep 18, 2023
ebf882e
test: tweak timeouts
ozyx Sep 18, 2023
84ba73a
chore: lint:fix
ozyx Sep 18, 2023
6acacc5
test: unskip perf tests
ozyx Sep 18, 2023
965f00d
fix: unregister events properly
ozyx Sep 19, 2023
3995ad5
fix: unregister listener
ozyx Sep 19, 2023
6c976e9
fix: unregister listener
ozyx Sep 19, 2023
31f2d8f
fix: unregister listener
ozyx Sep 19, 2023
6638b55
fix: use `unmounted()`
ozyx Sep 19, 2023
615c955
fix: unregister listeners
ozyx Sep 19, 2023
92eae6c
fix: unregister listener properly
ozyx Sep 19, 2023
0df79de
chore: lint:fix
ozyx Sep 19, 2023
79c4efb
test: fix imagery layer toggle test
ozyx Sep 19, 2023
ee95f9f
test: increase timeout
ozyx Sep 19, 2023
2741aa7
Don't use anonymous functions for listeners
shefalijoshi Sep 19, 2023
3a1e5dd
Destroy objects and event listeners properly
shefalijoshi Sep 19, 2023
a377f00
Delete config stores that are created by components
shefalijoshi Sep 19, 2023
b2a474a
Merge branch 'vue3-createApp-leak-fixes' of https://github.com/nasa/o…
shefalijoshi Sep 19, 2023
95a67ff
Use the right unmount hook. Destroy mounted view on unmount.
shefalijoshi Sep 19, 2023
6bcf026
Use unmounted, not beforeUnmounted
shefalijoshi Sep 19, 2023
eac81a4
Merge branch 'master' of https://github.com/nasa/openmct into vue3-cr…
shefalijoshi Sep 20, 2023
1321482
Lint fixes
shefalijoshi Sep 20, 2023
17505ea
Fix time strip memory leak
shefalijoshi Sep 20, 2023
81d4354
Undo unneeded change for memory leaks.
shefalijoshi Sep 20, 2023
d25be18
chore: combine common webpack configs
ozyx Sep 20, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,9 @@ jobs:
steps:
- build_and_install:
node-version: <<parameters.node-version>>
- run: npm run test:perf
- run: npm run test:perf:memory
- run: npm run test:perf:localhost
- run: npm run test:perf:contract
- store_test_results:
path: test-results/results.xml
- store_artifacts:
Expand Down
5 changes: 4 additions & 1 deletion .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,10 @@
"sinonjs",
"generatedata",
"grandsearch",
"websockets"
"websockets",
"swgs",
"memlab",
"devmode"
],
"dictionaries": ["npm", "softwareTerms", "node", "html", "css", "bash", "en_US"],
"ignorePaths": [
Expand Down
1 change: 1 addition & 0 deletions .webpack/webpack.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ const config = {
loader: 'vue-loader',
options: {
compilerOptions: {
hoistStatic: false,
whitespace: 'preserve',
compatConfig: {
MODE: 2
Expand Down
12 changes: 11 additions & 1 deletion .webpack/webpack.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,15 @@ module.exports = merge(common, {
__OPENMCT_ROOT_RELATIVE__: '""'
})
],
devtool: 'source-map'
devtool: 'source-map',
devServer: {
client: {
progress: true,
overlay: {
// Disable overlay for runtime errors.
// See: https://github.com/webpack/webpack-dev-server/issues/4771
runtimeErrors: false
}
}
}
});
14 changes: 9 additions & 5 deletions e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,11 @@ npm run test:e2e:updatesnapshots

## Performance Testing

The open source performance tests function mostly as a contract for the locator logic, functionality, and assumptions will work in our downstream, closed source test suites.
The open source performance tests function in three ways which match their naming and folder structure:

They're found under `./e2e/tests/performance` and are to be executed with the following npm script:

`npm run test:perf`
`./e2e/tests/performance` - The tests at the root of this folder path detect functional changes which are mostly apparent with large performance regressions like [this](https://github.com/nasa/openmct/issues/6879). These tests run against openmct webpack in `production-mode` with the `npm run test:perf:localhost` script.
`./e2e/tests/performance/contract/` - These tests serve as [contracts](https://martinfowler.com/bliki/ContractTest.html) for the locator logic, functionality, and assumptions will work in our downstream, closed source test suites. These tests run against openmct webpack in `dev-mode` with the `npm run test:perf:contract` script.
`./e2e/tests/performance/memory/` - These tests execute memory leak detection checks in various ways. This is expected to evolve as we move to the `memlab` project. These tests run against openmct webpack in `production-mode` with the `npm run test:perf:memory` script.

These tests are expected to become blocking and gating with assertions as we extend the capabilities of Playwright.

Expand All @@ -158,8 +158,11 @@ Our file structure follows the type of type of testing being excercised at the e
|`./tests/functional/example/` | Tests which specifically verify the example plugins (e.g.: Sine Wave Generator).|
|`./tests/functional/plugins/` | Tests which loosely test each plugin. This folder is the most likely to change. Note: some `@snapshot` tests are still contained within this structure.|
|`./tests/framework/` | Tests which verify that our testing framework's functionality and assumptions will continue to work based on further refactoring or Playwright version changes (e.g.: verifying custom fixtures and appActions).|
|`./tests/performance/` | Performance tests.|
|`./tests/performance/` | Performance tests which should be run on every commit.|
|`./tests/performance/contract/` | A subset of performance tests which are designed to provide a contract between the open source tests which are run on every commit and the downstream tests which are run post merge and with other frameworks.|
|`./tests/performance/memory` | A subset of performance tests which are designed to test for memory leaks.|
|`./tests/visual/` | Visual tests.|
|`./tests/visual/component/` | Visual tests which are only run against a single component.|
|`./appActions.js` | Contains common methods which can be leveraged by test case authors to quickly move through the application when writing new tests.|
|`./baseFixture.js` | Contains base fixtures which only extend default `@playwright/test` functionality. The expectation is that these fixtures will be removed as the native Playwright API improves|

Expand All @@ -176,6 +179,7 @@ Open MCT is leveraging the [config file](https://playwright.dev/docs/test-config
|`./playwright-ci.config.js` | Used when running in CI or to debug CI issues locally|
|`./playwright-local.config.js` | Used when running locally|
|`./playwright-performance.config.js` | Used when running performance tests in CI or locally|
|`./playwright-performance-devmode.config.js` | Used when running performance tests in CI or locally|
|`./playwright-visual.config.js` | Used to run the visual tests in CI or locally|

#### Test Tags
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,32 @@
// playwright.config.js
// @ts-check

const CI = process.env.CI === 'true';

/** @type {import('@playwright/test').PlaywrightTestConfig} */
const config = {
retries: 1, //Only for debugging purposes for trace: 'on-first-retry'
testDir: 'tests/performance/',
testMatch: '*.contract.perf.spec.js', //Run everything except contract tests which require marks in dev mode
timeout: 60 * 1000,
workers: 1, //Only run in serial with 1 worker
webServer: {
command: 'npm run start', //coverage not generated
command: 'npm run start', //need development mode for performance.marks and others
url: 'http://localhost:8080/#',
timeout: 200 * 1000,
reuseExistingServer: !CI
reuseExistingServer: false
},
use: {
browserName: 'chromium',
baseURL: 'http://localhost:8080/',
headless: CI, //Only if running locally
ignoreHTTPSErrors: true,
headless: true,
ignoreHTTPSErrors: false, //HTTP performance varies!
screenshot: 'off',
trace: 'on-first-retry',
video: 'off'
},
projects: [
{
name: 'chrome',
testIgnore: '*.memory.perf.spec.js', //Do not run memory tests without proper flags. Shouldn't get here
use: {
browserName: 'chromium'
}
Expand Down
60 changes: 60 additions & 0 deletions e2e/playwright-performance-prod.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/* eslint-disable no-undef */
// playwright.config.js
// @ts-check

/** @type {import('@playwright/test').PlaywrightTestConfig} */
const config = {
retries: 0, //Only for debugging purposes for trace: 'on-first-retry'
testDir: 'tests/performance/',
testIgnore: '*.contract.perf.spec.js', //Run everything except contract tests which require marks in dev mode
timeout: 60 * 1000,
workers: 1, //Only run in serial with 1 worker
webServer: {
command: 'npm run start:prod', //Production mode
url: 'http://localhost:8080/#',
timeout: 200 * 1000,
reuseExistingServer: false //Must be run with this option to prevent dev mode
},
use: {
baseURL: 'http://localhost:8080/',
headless: true,
ignoreHTTPSErrors: false, //HTTP performance varies!
screenshot: 'off',
trace: 'on-first-retry',
video: 'off'
},
projects: [
{
name: 'chrome-memory',
testMatch: '*.memory.perf.spec.js', //Only run memory tests
use: {
browserName: 'chromium',
launchOptions: {
args: [
'--no-sandbox',
'--disable-notifications',
'--use-fake-ui-for-media-stream',
'--use-fake-device-for-media-stream',
'--js-flags=--no-move-object-start --expose-gc',
'--enable-precise-memory-info',
'--display=:100'
]
}
}
},
{
name: 'chrome',
testIgnore: '*.memory.perf.spec.js', //Do not run memory tests without proper flags
use: {
browserName: 'chromium'
}
}
],
reporter: [
['list'],
['junit', { outputFile: '../test-results/results.xml' }],
['json', { outputFile: '../test-results/results.json' }]
]
};

module.exports = config;
1 change: 1 addition & 0 deletions e2e/test-data/memory-leak-detection.json

Large diffs are not rendered by default.

121 changes: 0 additions & 121 deletions e2e/tests/performance/memleak-imagery.perf.spec.js

This file was deleted.

Loading