-
Notifications
You must be signed in to change notification settings - Fork 30
Description
Address this code TODO:
cylc-ui/src/services/workflow.service.js
Lines 434 to 442 in 2902e5d
| // TODO: recompute, unsubscribe and wipe unwanted store data | |
| // see https://github.com/cylc/cylc-ui/issues/1131 | |
| // * The subscription has changed, there may be data we no longer need | |
| // * Recompute the subscription. | |
| // * Reload the subscription if needed. | |
| // * Remove objects requested before which are no longer needed. | |
| // this.recompute(subscription) | |
| // this.startSubscriptions() | |
| // store.commit(...) |
Note, the originally liked issue (#1131) doesn't quite cover this, so opening a new issue.
Explanation
Currently:
- If we start with a tree view.
- Then open the graph view.
- Then close the graph view.
The subscription is updated at (2) to merge in the requirements of the graph view, however, the subscription is not updated at (3) to remove these requirements.
This makes it faster to remove a view (we don't have to re-issue the subscription and re-build the store), however, does mean we leave behind data which may, subsequently become isolated from it's "prune" housekeeping, i.e, a memory leak.
This status quo has existed for a long time as the consequences of this are not generally too significant. We had anticipated the subscription logic developing much more than it actually needed to in practice (requested data turned out to be more lightweight than expected, delta processing more expensive, so fancy subscription handling hit the backburner).