Skip to content

perf: temporary subscriptions are properly unsubscribed#1704

Merged
TheSlimvReal merged 7 commits into
masterfrom
subscription_leaks
Feb 8, 2023
Merged

perf: temporary subscriptions are properly unsubscribed#1704
TheSlimvReal merged 7 commits into
masterfrom
subscription_leaks

Conversation

@TheSlimvReal

@TheSlimvReal TheSlimvReal commented Feb 6, 2023

Copy link
Copy Markdown
Contributor

I went through the suggestions in #1702 and implemented the ones that are actually problematic.
The following were left untouched:

  • HTTP observables are closed once completed
  • Route params subscriptions are cleaned up once user routes away (see this post)
  • Services that are singletons

Visible/Frontend Changes

--

Architectural/Backend Changes

  • cleaning up subscriptions that are not needed any more

@github-actions

github-actions Bot commented Feb 6, 2023

Copy link
Copy Markdown
Contributor

Deployed to https://pr-1704.aam-digital.net/

sleidig
sleidig approved these changes Feb 7, 2023
View reviewed changes

@sleidig sleidig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Hoping this will help general performance already 😊

Can we manage to rerun the tool of @Arooba-git and quantify the improvements?

And let's add her Co-authored-by: in this PR.

Comment thread src/app/core/form-dialog/form-dialog.service.ts
@TheSlimvReal

TheSlimvReal commented Feb 8, 2023

Copy link
Copy Markdown
Contributor Author

I ran the tool again, the results are vary significantly though. I does find fewer memory leaks but the actual memory usage does not change drastically/constantly.

@TheSlimvReal TheSlimvReal reopened this Feb 8, 2023
@github-actions

github-actions Bot commented Feb 8, 2023

Copy link
Copy Markdown
Contributor

Deployed to https://pr-1704.aam-digital.net/

@sonarqubecloud

sonarqubecloud Bot commented Feb 8, 2023

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@TheSlimvReal TheSlimvReal merged commit 9a64777 into master Feb 8, 2023
@TheSlimvReal TheSlimvReal deleted the subscription_leaks branch February 8, 2023 14:11
@aam-digital-ci

aam-digital-ci commented Feb 8, 2023

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 3.18.0-master.10 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@TheSlimvReal TheSlimvReal mentioned this pull request Feb 8, 2023
Closed
@aam-digital-ci

aam-digital-ci commented Feb 13, 2023

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 3.18.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sleidig sleidig sleidig approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@TheSlimvReal @aam-digital-ci @sleidig @Arooba-git