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

Properly propagate clones #8131

Closed
wants to merge 3 commits into from
Closed

Conversation

danog
Copy link
Collaborator

@danog danog commented Jun 21, 2022

This fixes more storage corruption issues I've detected by making stored Unions immutable.

@danog danog changed the title [release:fix] Properly propagate clones [Properly propagate clones Jun 21, 2022
@danog danog changed the title [Properly propagate clones Properly propagate clones Jun 21, 2022
@orklah
Copy link
Collaborator

orklah commented Jun 21, 2022

Seems good :) Could you try running a before/after to see the impact on memory usage?

@kkmuffme
Copy link
Contributor

@danog you just need to run psalm on any project of your choice once with and once without your changes; the output contains a line like this at the end:

Checks took 10.60 seconds and used 109.769MB of memory

If you could post it here for before/after we could get this merged.

@danog
Copy link
Collaborator Author

danog commented Sep 22, 2022

I actually started working on the immutability PRs that should supersede this one, I should actually close this :)

@danog danog closed this Sep 22, 2022
@kkmuffme
Copy link
Contributor

But since this is done, we could at least merge this and have some of the benefits now, while you're still working on the other PR?

@orklah
Copy link
Collaborator

orklah commented Sep 23, 2022

This will go on master and I'm waiting for the immutable PR to release the master branch so it won't come quicker anyway

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

Successfully merging this pull request may close these issues.

3 participants