Search the texture cache for small textures by address and hash #2097

mimimi085181 · 2015-02-22T17:46:11Z

This fixes issue 6563:
https://code.google.com/p/dolphin-emu/issues/detail?id=6563

This PR adds a 2nd map to texture cache, which uses the hash as key. Cache entries from this new map are used only if the address matches or if the texture was fully hashed. This restriction avoids false positive cache hits. This results in a possible situation where safe texture cache accuracy could be faster than the fast one.

Small textures means up to 1KB for fast texture cache accuracy, 4KB for medium, and all textures for safe accuracy.

Since this adds a small overhead to all texture cache handling, some regression testing would be nice. Games, which use a lot of textures the same time, should be affected the most.

Source/Core/VideoCommon/TextureCacheBase.cpp

-	std::pair <TexCache::iterator, TexCache::iterator> iter_range = textures.equal_range(address);
+	std::pair <TexCache::iterator, TexCache::iterator> iter_range;
+	if (g_ActiveConfig.iSafeTextureCache_ColorSamples == 0 || 
+		(texture_size <= (u32)g_ActiveConfig.iSafeTextureCache_ColorSamples * 8 && palette_size <= (u32)g_ActiveConfig.iSafeTextureCache_ColorSamples * 8))


delroth · 2015-02-22T17:56:00Z

@dolphin-emu-bot rebuild

Source/Core/VideoCommon/TextureCacheBase.cpp

@@ -342,7 +342,20 @@ TextureCache::TCacheEntryBase* TextureCache::Load(const u32 stage)
 	//
 	// For efb copies, the entry created in CopyRenderTargetToTexture always has to be used, or else it was
 	// done in vain.
-	std::pair <TexCache::iterator, TexCache::iterator> iter_range = textures.equal_range(address);
+	std::pair <TexCache::iterator, TexCache::iterator> iter_range;
+	if (g_ActiveConfig.iSafeTextureCache_ColorSamples == 0 || 


magumagu · 2015-02-22T17:59:17Z

@dolphin-emu-bot rebuild

Scanning the entire texture cache every time a small texture is bound is going to get really expensive in some cases.

delroth · 2015-02-22T18:06:11Z

@dolphin-emu-bot rebuild

mimimi085181 · 2015-02-22T18:09:54Z

Is there a better alternative than adding a 2nd map? Going through 500 entries for every texture is indeed bad. But keeping a 2nd map around for this special case bloats the code a bit. One alternative could be an option for this, but i want to avoid adding new options, unless it's really necessary.

Source/Core/VideoCommon/TextureCacheBase.cpp

-	std::pair <TexCache::iterator, TexCache::iterator> iter_range = textures.equal_range(address);
+	std::pair<TexCache::iterator, TexCache::iterator> iter_range;
+	if (g_ActiveConfig.iSafeTextureCache_ColorSamples == 0 ||
+		(texture_size <= (u32)g_ActiveConfig.iSafeTextureCache_ColorSamples * 8 && palette_size <= (u32)g_ActiveConfig.iSafeTextureCache_ColorSamples * 8))


mimimi085181 · 2015-04-21T12:11:37Z

Ok, i've optimised searching the texture cache:

Look for the texture by address 1st, this is the common case that happens almost all the time
Don't do anything different when the texture is not fully hashed
Reduced search loop to one if statement
Changed (tex_hash ^ tlut_hash) to full_hash. Not sure if the compiler optimises something like this, but doing the same 100 times in a loop seemed like a bad idea.

So in short, only if a texture is not found in the cache by address, the whole texture cache is searched. So the basic problem remains, and the worst case is still searching n entries n times each frame. But if a texture is not found in the cache, that's the general worst case already anyways. I'm not sure if this is really a big problem.

One other problem on top of that is that if a texture is not at the same address, it will be found in the cache, but Dolphin will always do the long search for it. Is there a fast way to clone a texture cache entry? Or a good way to link from one to another?

Source/Core/VideoCommon/TextureCacheBase.cpp

+	{
+		iter = textures.begin();
+
+		while (iter != textures.end())


degasus · 2015-05-02T20:59:14Z

But this c++11 loop, LGTM

mimimi085181 · 2015-05-09T12:45:54Z

So, i've added the 2nd map to find cache entries by their hash. It seems to be working well, but there's some cleanup to be done. The comments needs to be reworked, the names with 2 in them can't obviously stay, and the code to remove entries from the 2nd map needs to be put in a function or something.

Short: Do not review this yet, i'm still working on it.

mimimi085181 · 2015-05-09T18:42:07Z

Ok, now the PR is in a reviewable state.

Source/Core/VideoCommon/TextureCacheBase.cpp

+	// Remove the cache entry for normal textures from textures_by_hash and textures_by_address
+	if (!t_iter->second->IsEfbCopy())
+	{
+		std::pair<TexCache::iterator, TexCache::iterator> iter_range = textures_by_hash.equal_range(t_iter->second->hash);


mimimi085181 · 2015-05-10T10:51:21Z

How can i write "std::multimap<u64, TCacheEntryBase*>::iterator textures_by_hash_iter;" as "TexCache::iterator textures_by_hash_iter;" without changing too many lines?

I think the performance can't be improved much further. So it's time to clean up the coding style and names, which i suck at, sorry.

Source/Core/VideoCommon/TextureCacheBase.cpp

+{
+	if (iter->second->textures_by_hash_iter != textures_by_address.end())
+	{
+		textures_by_hash.erase(iter->second->textures_by_hash_iter);


This fixes issue 6563: https://code.google.com/p/dolphin-emu/issues/detail?id=6563 This PR adds a 2nd map to texture cache, which uses the hash as key. Cache entries from this new map are used only if the address matches or if the texture was fully hashed. This restriction avoids false positive cache hits. This results in a possible situation where safe texture cache accuracy could be faster than the fast one. Small textures means up to 1KB for fast texture cache accuracy, 4KB for medium, and all textures for safe accuracy. Since this adds a small overhead to all texture cache handling, some regression testing would be nice. Games, which use a lot of textures the same time, should be affected the most.

mimimi085181 · 2015-06-05T19:51:25Z

So does this have a chance to be merged? Or is the lack of reviews related to how horrible the code is?

Without the PR, in Tales of Symphonia gamecube, some menus are slower than the gameplay, world map, fights etc. So on low end machines that barely run the game, the menus will lag. Is avoiding this worth making the code more complicated?

Or should the texture cache get a major overhaul of some kind anyways? Maybe something like allowing countless elements and only removing them when they haven't been used for a long time, and memory is getting low?

JMC47 · 2015-06-05T20:01:15Z

@dolphin-emu-bot rebuild

Search the texture cache for small textures by address and hash

Code cleanup for FreeTexture after merging PR #2097

FioraAeterna reviewed Feb 22, 2015
View reviewed changes

delroth reviewed Feb 22, 2015
View reviewed changes

mimimi085181 force-pushed the find-textures-by-hash branch from 92c52d1 to b8dca24 Compare February 22, 2015 18:01

degasus reviewed Feb 23, 2015
View reviewed changes

mimimi085181 force-pushed the find-textures-by-hash branch from b8dca24 to c389915 Compare March 20, 2015 18:26

mimimi085181 force-pushed the find-textures-by-hash branch 4 times, most recently from 36b233a to 6dd2a3a Compare April 21, 2015 12:07

degasus reviewed May 2, 2015
View reviewed changes

Source/Core/VideoCommon/TextureCacheBase.cpp

{

iter = textures.begin();

while (iter != textures.end())

This comment was marked as off-topic.

Sign in to view

mimimi085181 force-pushed the find-textures-by-hash branch from 6dd2a3a to 98bc6f5 Compare May 9, 2015 12:41

mimimi085181 force-pushed the find-textures-by-hash branch from 98bc6f5 to 18e384e Compare May 9, 2015 18:40

degasus reviewed May 10, 2015
View reviewed changes

mimimi085181 force-pushed the find-textures-by-hash branch 2 times, most recently from c418c9a to bf7a83b Compare May 10, 2015 10:46

degasus reviewed May 10, 2015
View reviewed changes

Source/Core/VideoCommon/TextureCacheBase.cpp

{

if (iter->second->textures_by_hash_iter != textures_by_address.end())

{

textures_by_hash.erase(iter->second->textures_by_hash_iter);

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

mimimi085181 force-pushed the find-textures-by-hash branch 2 times, most recently from c3d4ff7 to 9bbeba7 Compare May 10, 2015 11:23

mimimi085181 force-pushed the find-textures-by-hash branch from 9bbeba7 to 3b9020d Compare June 5, 2015 19:40

degasus added a commit that referenced this pull request Jun 9, 2015

Merge pull request #2097 from mimimi085181/find-textures-by-hash

e47e4c6

Search the texture cache for small textures by address and hash

degasus merged commit e47e4c6 into dolphin-emu:master Jun 9, 2015

mimimi085181 added a commit to mimimi085181/dolphin that referenced this pull request Jun 9, 2015

Code cleanup for FreeTexture after merging PR dolphin-emu#2097

4d5fdb7

degasus added a commit that referenced this pull request Jun 10, 2015

Merge pull request #2581 from mimimi085181/FreeTexture-code-cleanup

9b8eb55

Code cleanup for FreeTexture after merging PR #2097

mimimi085181 added a commit to mimimi085181/dolphin that referenced this pull request Jun 24, 2015

Code cleanup for FreeTexture after merging PR dolphin-emu#2097

c3dc532

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search the texture cache for small textures by address and hash #2097

Search the texture cache for small textures by address and hash #2097

mimimi085181 commented Feb 22, 2015

This comment was marked as off-topic.

delroth commented Feb 22, 2015

This comment was marked as off-topic.

magumagu commented Feb 22, 2015

delroth commented Feb 22, 2015

mimimi085181 commented Feb 22, 2015

This comment was marked as off-topic.

mimimi085181 commented Apr 21, 2015

This comment was marked as off-topic.

degasus commented May 2, 2015

mimimi085181 commented May 9, 2015

mimimi085181 commented May 9, 2015

This comment was marked as off-topic.

mimimi085181 commented May 10, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

mimimi085181 commented Jun 5, 2015

JMC47 commented Jun 5, 2015

Search the texture cache for small textures by address and hash #2097

Search the texture cache for small textures by address and hash #2097

Conversation

mimimi085181 commented Feb 22, 2015

This comment was marked as off-topic.

delroth commented Feb 22, 2015

This comment was marked as off-topic.

magumagu commented Feb 22, 2015

delroth commented Feb 22, 2015

mimimi085181 commented Feb 22, 2015

This comment was marked as off-topic.

mimimi085181 commented Apr 21, 2015

This comment was marked as off-topic.

degasus commented May 2, 2015

mimimi085181 commented May 9, 2015

mimimi085181 commented May 9, 2015

This comment was marked as off-topic.

mimimi085181 commented May 10, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

mimimi085181 commented Jun 5, 2015

JMC47 commented Jun 5, 2015