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

Use a minihash in the vertex cache, treat unreliable arrays more carefully #6855

Merged
merged 5 commits into from
Sep 13, 2014

Conversation

unknownbrackets
Copy link
Collaborator

The minihash should be fairly fast (unless many small flushes are pushed, but we're already in trouble there.) It seems to catch many cases for me, although not all. I've only benchmarked it on Windows, but it checks a small subset of the data.

Unreliable arrays were "expiring" 120 flips after they became unreliable. If a bunch were on the same frame, this would be a hit every 120 flips potentially all at once. So this also prevents expiring too many in the same flip, and extends the unreliable length.

For me this improves both the performance (slightly) and accuracy of the vertex cache.

-[Unknown]

@daniel229
Copy link
Collaborator

vertexcache issue looks fixed in these games
Assassin's Creed Bloodlines
Danbal Senki W
danganronpa 1&2
Indiana Jones and the Staff of Kings
Project Diva Extend
Toukiden series

God of war Chains of Olympus
Ikkitosen Xross Impact

Not fixed
Brave Story
Innocent Life
Final fantasy 4

@unknownbrackets
Copy link
Collaborator Author

If you change TransformPipeline.cpp, find:

                        if (newMiniHash != vai->minihash || newHash != vai->hash) {
                            MarkUnreliable(vai);
                            DecodeVerts();
                            goto rotateVBO;
                        }

To:

                        if (newMiniHash != vai->minihash || newHash != vai->hash) {
                            if (newHash != vai->hash) {
                                ERROR_LOG(G3D, "Hash failure %08x.  numDrawCalls=%d, numVerts=%d, inds=%d", id, numDrawCalls, vai->numVerts, (dec_->VertexType() & GE_VTYPE_IDX_MASK));
                            }
                            MarkUnreliable(vai);
                            DecodeVerts();
                            goto rotateVBO;
                        }

In the games that don't work, what does it log around the times the flickering occurs? I'm mostly interested in knowing if they are big or small (small could be hashed a bit more maybe), using indexes or not.

-[Unknown]

@daniel229
Copy link
Collaborator

Innocent Life seems no errors.

Final fantasy 4 seems is the biggest
https://gist.github.com/daniel229/7493339e4a863e49cc6a

Brave Story smaller
https://gist.github.com/daniel229/d94b8b4fda0a9f7e7117

@unknownbrackets
Copy link
Collaborator Author

Oh, that's interesting. Maybe Innocent Life is getting hit by a vertex cache bug or a hash collision, rather than just not checking often enough or reuse...

Hmm, Brave Story uses quite large ones. Could try changing:

    if (sz > 100) {
        size_t step = std::min(sz / 16, (size_t)32);
        u32 hash = 0;
        for (size_t i = 0; i < sz; i += step) {
            hash += DoReliableHash(p + i, 128, 0x3A44B9C4);
        }
        return hash;
    } else if (sz >= 4) {
        size_t step = sz / 4;
        u32 hash = 0;
        for (size_t i = 0; i < sz; i += step) {
            hash += p[i];
        }
        return hash + p[sz - 1];
    } else {
        return p[0] + p[sz - 1];
    }

That will has a bit more, so I'll have the bench it again, but probably still fast and probbly worth it if it helps.

-[Unknown]

@daniel229
Copy link
Collaborator

It does not help.

@daniel229
Copy link
Collaborator

Innocent Life might be other issues,since it's not right with vertics cache off.

@unknownbrackets
Copy link
Collaborator Author

Well, not sure if I can figure out how to detect those two without vertex cache off then, but at least this makes things better and so far nothing worse. Concerned a bit about Android benchmarks.

-[Unknown]

@hrydgard
Copy link
Owner

Seems good but unsure performance-wise if I should merge.. Maybe should take some time to bench a few games on android.

@unknownbrackets
Copy link
Collaborator Author

Senjou no Valkyria 3:
Char select (2D) - before: 220, after: 220
Movement - before: 208, after: 208

Dissidia, battle - before: 190, after: 190

Diva demo, selection - before: 245, after: 252 (+ she's blinking now)

Final Fantasy 3, battle - before: varies, 480? after: varies about as much, ~480

-[Unknown]

hrydgard added a commit that referenced this pull request Sep 13, 2014
Use a minihash in the vertex cache, treat unreliable arrays more carefully
@hrydgard hrydgard merged commit c21c432 into hrydgard:master Sep 13, 2014
@unknownbrackets unknownbrackets deleted the vertexcache branch September 14, 2014 04:12
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 this pull request may close these issues.

3 participants