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

Replace cached GSOffset with live calculations #4348

Merged
merged 12 commits into from
Nov 7, 2021

Conversation

TellowKrinkle
Copy link
Member

Should fix #2686, please test

The non-cached calculations are pretty fast, but generally not quite as fast as the cached versions. I haven't noticed any slowdown in anything I've tested, but it might be good for others to test.

Also this makes heavy use of lambdas hoping they'll be inlined, so we should performance test MSVC and make sure it actually does

Finally, we haven't used them much before, so what are people's thoughts on formatting lambdas passed to functions? I went with this (indented like you would the content of a loop):

s->m_pages.loopPages([this, s](uint32 page)
{
	s->m_erase_it[page] = m_map[page].InsertFront(s);
});

but we could also go with this

s->m_pages.loopPages([this, s](uint32 page)
	{
		s->m_erase_it[page] = m_map[page].InsertFront(s);
	});

or this

s->m_pages.loopPages(
	[this, s](uint32 page)
	{
		s->m_erase_it[page] = m_map[page].InsertFront(s);
	});

I personally would like to not go with clang-format's preference of this, it destroys horizontal space

s->m_pages.loopPages([this, s](uint32 page)
                     {
                         s->m_erase_it[page] = m_map[page].InsertFront(s);
                     });

Same question for locals

auto helper = [&](...)
{
};

or

auto helper = [&](...)
	{
	};

@TellowKrinkle TellowKrinkle force-pushed the GSLocalMemCleanup branch 4 times, most recently from 505d14e to bc3d50e Compare April 2, 2021 10:46
@seta-san
Copy link
Contributor

seta-san commented Apr 2, 2021

tour de france doesn't have memory leak anymore. memory usage doesn't get much higher than 560mb. characters and other UI elements flicker in and out of existence which is new (incorrect) behavior.

https://youtu.be/msNw_BPzrAs

@seta-san
Copy link
Contributor

seta-san commented Apr 2, 2021

multiframe dump.
https://filebin.net/dham5zxe2g89mb8e

@TellowKrinkle TellowKrinkle force-pushed the GSLocalMemCleanup branch 3 times, most recently from 0a6064b to 57a4e95 Compare April 3, 2021 10:17
@TellowKrinkle
Copy link
Member Author

@seta-san try now

@seta-san
Copy link
Contributor

seta-san commented Apr 3, 2021

@seta-san try now

works perfect now.

@MrPopoJunior
Copy link

Is this fix gonna be included in a dev build of PCSX2?

@F0bes
Copy link
Member

F0bes commented Apr 3, 2021

Once the PR is merged into master, it will be in the latest dev build yes.

@TellowKrinkle TellowKrinkle force-pushed the GSLocalMemCleanup branch 3 times, most recently from 1315f18 to ad004a5 Compare April 3, 2021 22:49

unsigned long j;

while(_BitScanForward(&j, p))
Copy link
Contributor

Choose a reason for hiding this comment

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

This was a micro optimization from greg for reducing the source cache invalidation time with the old page validity bitmap; It might be a good benchmark to compare the time spent in the GSTextureCache::SourceMap::RemoveAt before and after the modifications.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm the two commits that modified it before were fa1377a and e58776e

fa1377a looks like it was adding a caching system that's since been replaced, though it does name Champion of Norrath as the game to profile for this section of code

e58776e is the one that adds the _BitScanForward and doesn't mention what games it's trying to improve

So does anyone have Champion of Norrath (or a gsdump of it) to profile?

Copy link
Member

Choose a reason for hiding this comment

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

Champions of Norrath is just a Snowblind engine game, so any game using that, for example Baldurs Gate, will probably do.

Copy link
Contributor

Choose a reason for hiding this comment

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

My memory has betrayed me about the git history, sorry. Anyway I tested the beginning of Baldur's Gate: Dark Alliance, and it is even a tad faster now! Great job.

Choose a reason for hiding this comment

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

Just an update, since it's been a few months since my last comment on this build. I was able to complete Remote Control Dandy SF 100% using this build. I only encountered some geometry issues during cutscenes, but everything was near-perfect during gameplay. The only other issue I had was some random crashes which happened hours apart from eachother, so I'm attributing those to the nature of this being an experimental PR build.

@ThatDudeYouMetThatOneTime

This build seems to have fixed the issue with Remote Control Dandy SF, which would cause the game to crash after about 10 to 15 minutes of playing. In my testing thus far, I have been able to play the game for an hour and 10 minutes without experiencing any crashes. If anyone else would be interested in testing RCD SF with this build as well, and for a longer period of time, please feel free to do so.

@seta-san
Copy link
Contributor

seta-san commented Apr 5, 2021

This build seems to have fixed the issue with Remote Control Dandy SF, which would cause the game to crash after about 10 to 15 minutes of playing. In my testing thus far, I have been able to play the game for an hour and 10 minutes without experiencing any crashes. If anyone else would be interested in testing RCD SF with this build as well, and for a longer period of time, please feel free to do so.

ANOTHER KONAMI GAME

@RedDevilus
Copy link
Contributor

Ultimate Spider-man doesn't seem to crash anymore even after 30 min, though the game suffers from other problems like texture cache.

@seta-san
Copy link
Contributor

seta-san commented Apr 6, 2021

this may also effect "Anubis: Zone of Enders"

from 5chan
658名無しさん@お腹いっぱい。 (ワッチョイ 9b62-0pr0)2021/04/06(火) 11:13:37.76ID:+rNq7+UQ0
anubisのメモリリークも解消されますように(May anubis memory leaks be eliminated.)

@TellowKrinkle TellowKrinkle force-pushed the GSLocalMemCleanup branch 2 times, most recently from 83cd83a to ba110a1 Compare April 6, 2021 22:38
@TellowKrinkle
Copy link
Member Author

Speaking of 5ch

試したいけどgithub使ったことがなくてやり方が分からない

  1. 右上のSign Upのボタンで無料でgithubのアカウントを作ろう。
  2. 緑色の✔︎をクリックして、Windows buildの右のDetailsと言うボタンをクリック
    image
  3. ログインしているならArtifactsが表示される。そこでPCSX2-32bit-pr...をダウンロードする
    image
  4. 問題があれば公式Discordサーバー#help-windowsチャンネルで聞いてください。日本語で聞いても時間がちょっとかかるが日本語のできる人がきっと手伝うよ

Copy link
Member

@F0bes F0bes left a comment

Choose a reason for hiding this comment

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

Tested with Colin McRae Rally 3 and Grand Theft Auto: LCS

Both seem to work fine, CM Rally seemed to be a couple of fps faster
GTA: LCS seemed to be a touch slower

The difference is very minuscule and could just external factors.

@ghost
Copy link

ghost commented Apr 10, 2021

This PR produces visual glitches on Zone of the enders the 2nd runner demo.

image

@TellowKrinkle
Copy link
Member Author

This PR produces visual glitches on Zone of the enders the 2nd runner demo.

Send a GSdump please

@TellowKrinkle TellowKrinkle force-pushed the GSLocalMemCleanup branch 5 times, most recently from 8158184 to cca632a Compare October 14, 2021 04:17
@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Oct 22, 2021

Not really a bug but an observation, there's a 2D screen after the main menu of Drakengard which on Master starts around 70fps and then drops down to 40fps after a few seconds (toggling F9 brings it back up for a short while), however on this PR it is constantly at 40fps no matter what. This is obviously evidence of the game spamming crap to flush the draws or something, but I figured it interesting to mention.

I should note that Software mode sits around 60-90fps

@Rasiohead91
Copy link

how to install this update on the pcsx2? I am just rookie

@Mrlinkwii
Copy link
Contributor

@Rasiohead91 go to https://pcsx2.net/downloads/ and scroll down to where you see 1.7.27XX , if you have any other issues join the discord/ forums thats linked in the readme :) ,

@Rasiohead91
Copy link

@Rasiohead91 go to https://pcsx2.net/downloads/ and scroll down to where you see 1.7.27XX , if you have any other issues join the discord/ forums thats linked in the readme :) ,

3q very much,and the latest version is the best?

@RedDevilus
Copy link
Contributor

@Rasiohead91 go to pcsx2.net/downloads and scroll down to where you see 1.7.27XX , if you have any other issues join the discord/ forums thats linked in the readme :) ,

3q very much,and the latest version is the best?

I would assume having a big ass button would tell you that yes, the latest version is the best.

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

Successfully merging this pull request may close these issues.

GS: MLB Power Pros crash