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

Greg/gsdx boost texturecache #1766

Merged
merged 3 commits into from Jan 15, 2017

Conversation

Projects
None yet
5 participants
@gregory38
Contributor

gregory38 commented Jan 12, 2017

Greatly reduce the texture removal operation overhead. It might help to improve GSdx speed a little.

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

error C2079: 'GSTextureCache::Source::m_erase_it' uses undefined class 'std::array<std::_List_iterator<std::_List_val<std::_List_simple_types<GSTextureCache::Source *>>>,512>'

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 12, 2017

😭 I don't know what is the issue (except that VS sucks).

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

I'm on it don't worry :P I'll commit it when I fix it

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

array<list<Source*>::iterator, MAX_PAGES> m_erase_it;

saying "incomplete type not allowed"

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

also

"cannot deduce 'auto' type" on line 1989 and 2091 in texturecache

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 12, 2017

auto will work when m_erase_it is properly detected. What happen if you remove the ::iterator does it manage to detect the m_erase_it type (obviously it will fail later).

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

Nope but if i make it a pointer it works

array<list<Source*>::iterator, MAX_PAGES> *m_erase_it;

the auto thing is still pissy though :P

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 12, 2017

Otherwise try to forward declare Source before m_erase_it.

class Source;
blabla m_erase_it;

or class GSTextureCache::Source

edit: add the ;

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

Otherwise try to forward declare Source before m_erase_it.

What? lol

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 12, 2017

See example above. Just telling the compiler that Source is a type which might not be yet true inside Source.

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

I don't understand your example... what exactly do you want me to do?

@gregory38

This comment has been minimized.

@turtleli

This comment has been minimized.

Member

turtleli commented Jan 12, 2017

? Not sure how this even compiles on Linux. I think the <array> include is missing?

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 12, 2017

Ah it must be it. I guess it depends on the order of the include (or issue is include from Dx).

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 12, 2017

let's see if it is better.

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

it compiles here now :)

I'm glad because I still didn't have a clue what you wanted me to do lol

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 12, 2017

Great. Conclusion, Linux is much better for dev. You give garbage, and it replies with a working program. VS choose the reverse way 😛

Now, let's hope perf is better !

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

It's not like Linux not to be anal about something... :P Very out of character!

Going to test a few games now Vs Master

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 12, 2017

(I hope it isn't slower). The speed impact might be bigger on game that are heavy on texture such as Zone of Ender, baldur gate.

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

With MTVU off, Baldurs gate seems possibly 2-5fps faster. so I'm hitting 335fps roughly, instead of 330fps

similar increases with MTVU on

Driv3r is 1-2fps faster
Ratchet & Clank maybe 1fps faster
Black "might" be 1-2fps faster? I can't tell.

I did test a bunch of other games but didn't really notice a difference.

@lightningterror

This comment has been minimized.

Member

lightningterror commented Jan 12, 2017

2-5 fps is a good improvement.

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 12, 2017

Faster is still better than slower. Besides it is more robust and might allow to fix some bugs.
It would have been nice to test the Anubis boss on ZoE. Maybe it will be less slow.

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 12, 2017

Sorry I don't have ZoE

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 12, 2017

No worry. Maybe someone will shine.

@gregory38 gregory38 force-pushed the greg/gsdx-boost-texturecache branch Jan 13, 2017

@gregory38

This comment has been minimized.

Contributor

gregory38 commented Jan 13, 2017

Could this PR be merged ? So I can continue the work on the GS memory wrapping (#1748)

@refractionpcsx2

This comment has been minimized.

Member

refractionpcsx2 commented Jan 13, 2017

It seems to work, so sure :)

plugins/GSdx/GSTextureCache.cpp Outdated
// FIXME: this statement could be optimized to a single ASM instruction (instead of 4)
// Either BTR (AKA bit test and reset). Depends on the previous instruction.
// Or BLSR (AKA Reset Lowest Set Bit). No dependency but require BMI1 (basically a recent CPU)
p ^= 1 << j;

This comment has been minimized.

@turtleli

turtleli Jan 13, 2017

Member

I suppose I wonder whether this should actually be p ^= 1U << j; (I think 1 << 31 is undefined behaviour, but 1U << 31 is not). EDIT: 1 << 31 is undefined in C++11, but defined in C++14.

As for merging - You can go ahead, though I kinda want std:: to be prefixed onto array and list ;)

gregory38 added some commits Jan 12, 2017

gsdx tc: save list iterator to allow fast removal
ZoE2:
RemoveAt overhead plummet to 0.5%. It was 17% !

However insertion is a bit slower. Due to the begin() after the push_front

v2: use std:: for lists and arrays
gsdx tc: implement a safe RemoveAt
The code is now a mirror of the ::add. So 1 insert == 1 erase

This way it won't crash on future update. And it will support future GS
memory wrapping improvement.
gsdx tc: avoid any pitfall with 1 << 31
Based on Turtleli feedback
"1 << 31 is undefined in C++11, but defined in C++14"

@gregory38 gregory38 force-pushed the greg/gsdx-boost-texturecache branch to 92c0470 Jan 15, 2017

@gregory38 gregory38 merged commit f9c2025 into master Jan 15, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@gregory38 gregory38 deleted the greg/gsdx-boost-texturecache branch Jan 15, 2017

@MarcoEstevez

This comment has been minimized.

MarcoEstevez commented Jan 16, 2017

Good job

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