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

Place pinned memory as top priority. #555

Merged
merged 1 commit into from Jul 4, 2014

Conversation

JMC47
Copy link
Contributor

@JMC47 JMC47 commented Jul 4, 2014

Was doing some testing with buffer storage, coherent mapping, and pinned memory, and found out that Pinned Memory was faster on AMD cards even with coherent mapping on. As such, if we move Pinned Memory to the first priority, it makes both NVIDIA and AMD get maximum performance.

On my Radeon HD5850, this is an 8% boost compared to before the performance regression. This should probably be tested on a variety of graphics cards even though it's a minor change. I do know it works fine on my GTX 760 and my Radeon HD5850.

Implementation is based on what Sonicadvance1 told me to do plus the fact only AMD supports Pinned Memory, no one else.

@Sonicadvance1
Copy link
Contributor

@dolphin-emu-bot rebuild

@Sonicadvance1
Copy link
Contributor

@dolphin-emu-bot rebuild

@JMC47 JMC47 changed the title Places pinned memory as top priority. Place pinned memory as top priority. Jul 4, 2014
@Anti-Ultimate
Copy link
Contributor

Merge asap, amd people are already complaining about bad performance on the forums 👍

@degasus
Copy link
Member

degasus commented Jul 4, 2014

It's sad that coherent mapping is slower than pinned memory as it's basicly the same :/
But it isn't our job to know what's wrong with the gfx driver ;)

LGTM

@neobrain
Copy link
Member

neobrain commented Jul 4, 2014

Comment needs to be clarified, otherwise LGTM.

return new BufferStorage(type, size);

// pinned memory is almost as fine
// pinned memory is much faster on amd cards

This comment was marked as off-topic.

delroth added a commit that referenced this pull request Jul 4, 2014
Place pinned memory as top priority.
@delroth delroth merged commit 7035e5e into dolphin-emu:master Jul 4, 2014
@Nucleoprotein
Copy link

On my HD6850 performance seems to be exactly same as using buffer_storage, maybe you tested old catalysts (i use latest 14.6 RC). It would be better to add this as configurable option...

@JMC47
Copy link
Contributor Author

JMC47 commented Jul 5, 2014

Regardless of that, many others have said that Buffer Storage + Coherent Mapping off was extremely slow. Pinned Memory being used fixes that, without needing a hack for AMD video cards.

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