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

Don't permalloc the pkgimgs, but add option for PkgCacheInspector #49940

Merged
merged 1 commit into from
May 25, 2023

Conversation

gbaraldi
Copy link
Member

@gbaraldi gbaraldi commented May 23, 2023

This saves some memory when loading the package images.
The option restores the current behavior for packages like https://github.com/timholy/PkgCacheInspector.jl to keep working.

This is a rebase of #48215

We might want to backport this to 1.9

@gbaraldi gbaraldi requested a review from vchuravy May 23, 2023 19:53
@oscardssmith oscardssmith added the performance Must go faster label May 23, 2023
@vchuravy vchuravy requested a review from timholy May 24, 2023 11:04
@vchuravy vchuravy added the backport 1.9 Change should be backported to release-1.9 label May 25, 2023
@vchuravy
Copy link
Member

Nominated for backport to 1.9 since it reduces memory usage significantly.

@giordano
Copy link
Contributor

since it reduces memory usage significantly.

Do you have some numbers to share?

@vchuravy
Copy link
Member

Omnipackage v1.9.0

Kbytes RSS Dirty
7984776 3494156 2072436

1.10 with this PR

Kbytes RSS Dirty
7194300 2713000 1619908

Just Trixi

1.8

Kbytes RSS Dirty
3438692 744456 584264

1.9

Kbytes RSS Dirty
2353464 1040784 613748

1.10 with Patch

Kbytes RSS Dirty
2112344 831728 429724

using pmap -x for the measurement and doing using OmniPackage / using Trixi respectivly.

@vchuravy vchuravy merged commit 229269b into JuliaLang:master May 25, 2023
@oscardssmith
Copy link
Member

Can you add to the table 1.10 without the PR? (just for fairness)

@gbaraldi
Copy link
Member Author

I think you start seeing benefits if you load the same package in multiple processes, because with this we can share

@ranocha
Copy link
Member

ranocha commented May 29, 2023

Should this be backported to Julia v1.9.1?

@ranocha
Copy link
Member

ranocha commented May 29, 2023

I think you start seeing benefits if you load the same package in multiple processes, because with this we can share

If this is the case, it would be great for our HPC applications using MPI

@gbaraldi
Copy link
Member Author

It might be too late for 1.9.1. @KristofferC ?

@KristofferC
Copy link
Member

For 1.9.1 yes.

@ranocha
Copy link
Member

ranocha commented May 29, 2023

Thanks for the info - and for all the hard work on Julia, every new release, backports etc. I'm looking forward to v1.9.1 - and v1.9.2 including this fix 🙂

@KristofferC KristofferC mentioned this pull request Jun 6, 2023
36 tasks
KristofferC pushed a commit that referenced this pull request Jun 27, 2023
KristofferC pushed a commit that referenced this pull request Jun 27, 2023
KristofferC pushed a commit that referenced this pull request Jun 27, 2023
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Jul 10, 2023
@gbaraldi gbaraldi deleted the rebase-no-permalloc branch August 14, 2023 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants