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

Persona 3 memory leak loading texture replacments #16135

Closed
2 of 5 tasks
stuken opened this issue Sep 30, 2022 · 9 comments · Fixed by #16304
Closed
2 of 5 tasks

Persona 3 memory leak loading texture replacments #16135

stuken opened this issue Sep 30, 2022 · 9 comments · Fixed by #16304
Milestone

Comments

@stuken
Copy link
Contributor

stuken commented Sep 30, 2022

Game or games this happens in

ULUS-10512

What area of the game / PPSSPP

Persona 3 Portable appears to be causing a memory leak when loading in texture replacments.
An increase in ram usage can triggered by viewing the character status and holding L or R to cycle the pages. A few seconds of cycling will steadily increase allocated memory well beyond what happens with no replacements.

Occurs with all rendering backends.

Texture pack: https://forums.ppsspp.org/showthread.php?tid=23509

Save attached to go straight to reproduction area.
ULUS10512DATA00.zip

Logs show thousands of hits to https://github.com/hrydgard/ppsspp/blob/master/GPU/Common/TextureCacheCommon.cpp#L814

What should happen

Ram use should not increase when repeatedly displaying the same replacement texture.

Logs

ppsspplog.zip

Platform

Windows

Mobile phone model or graphics card

rtx3090

PPSSPP version affected

7a4830e

Last working version

No response

Graphics backend (3D API)

Vulkan

Checklist

  • Test in the latest git build in case it's already fixed.
  • Search for other reports of the same issue.
  • Try resetting settings or older versions and include if the issue is related.
  • Try without any cheats and without loading any save states.
  • Include logs or screenshots of issue.
@unknownbrackets
Copy link
Collaborator

From looking at a before/after snapshot and from quitting the game, this isn't really a "leak" it's just overeager memory usage, in part because the texture pack is so large. At least in the place you've provided as reproduction.

What I see is:

  • If you quit the game, memory usage normalizes (so nothing has leaked, it gets freed properly.)
  • If you wait 15 minutes, unused texture data gets flushed, and memory usage also normalizes a bit.

Which would imply that it's using gigabytes of RAM only because you sometimes have that much texture data loaded and cached at once. This is more obvious if you reduce the cache lifetime, here:

	// Allow replacements to be cached for a long time, although they're large.
	double age = 1800.0;
	if (mode == ReplacerDecimateMode::FORCE_PRESSURE)
		age = 90.0;
	else if (mode == ReplacerDecimateMode::ALL)
		age = 0.0;

Changing 1800.0 and 90.0 to 5.0 makes it difficult to reproduce any runaway memory usage, but of course means more late loading replacements. Loading and decoding the PNGs is pretty slow, which is why we cache them decompressed. But a 6x image is large: (512 * 6) * (512 * 6) * 4 bytes = 36 MB per image.

It's caching these per cache key (which can include noise from unrelated texture data packed into the 512x512 area), which I think was noted when adding the cache initially as a potential opportunity to reduce the cache memory use. So my guess is there's just somewhere where a lot of very large textures are being loaded from the same file, but subtly different cache keys. It's actually consuming VRAM and everything else too, because we kinda need to cache by that key. It just holds on to replaced textures the longest.

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Oct 1, 2022

For these absurdly high res texture packs, we probably should support some compressed texture formats like DXT, BC7, ETC2, would cut memory use to a quarter. Unfortunately most of those are not cross compatible between mobile and PC. There are compromise formats like Basis that can be converted to either quickly, but they're not quite as good and are not free to convert either...

I think we have an issue sitting around about this somewhere.. yeah, #12059 .

@anr2me
Copy link
Collaborator

anr2me commented Oct 2, 2022

Is this the issue a saw on discord where someone posted a screenshot of PPSSPP is using 10gb of memory? O.o that is such a large memory usage
image

@unknownbrackets
Copy link
Collaborator

That's actually only about 275 large 512x512 textures at 6x - not really hard to imagine within 15 minutes. As hrydgard said, "absurdly high res."

We could track how much memory the replacement cache is using to more aggressively clear it over some arbitrary threshold too.

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Oct 2, 2022

Although it's weird that you'd encounter that many different textures just browsing the menu, as in this example? Might we somehow be loading some textures multiple times?

@anr2me
Copy link
Collaborator

anr2me commented Oct 2, 2022

Having a limit of how much memory the cache use at max would be nice, this could avoid crashing other app (or background services) when they need memory.

@unknownbrackets
Copy link
Collaborator

Although it's weird that you'd encounter that many different textures just browsing the menu, as in this example? Might we somehow be loading some textures multiple times?

Right, as noted it doesn't currently deduplicate - it loads the texture data per cache key, not per filename. So if you have a range of cache keys pointing to the same file, it'll load the data for each one and keep it around for 15 minutes.

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Oct 2, 2022

Ah, I see, I missed that post above. I think we could have a second level of indirection there to avoid loading a file multiple times in such cases.

@hrydgard hrydgard added this to the v1.14.0 milestone Oct 3, 2022
@rwixl
Copy link

rwixl commented Oct 18, 2022

I literally have the exact same issue here, i don't even need to share more info about it in my end.

I tried downscaling the pack to x3 but that didn't solve the issue (I think it's because i am using a 3GB Ram phone).

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 a pull request may close this issue.

5 participants