Improve performance by fusing kernels#57
Open
au2001 wants to merge 5 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change Summary
What does this PR change?
It improves performance by 15-98% through 2 compounding changes:
mp_numberto 2x 16 bytes instead of 8x 4 bytes; and(
profanity_inverseis untouched in this PR, so there are still 2 launches total, down from 3-4)Testing & Verification
How was this tested?
I ran each scoring method on different GPUs with the previous and new codebase.
For each resulting address, I ran an external verification script which found no mistake.
Here are the median results for the ~most popular variant of each NVIDIA generation:
Notes:
GPUs which were bound by memory latency/bandwidth see the most performance increase.
That's consumer-grade GPUs, while the L40S sees the least gain, as expected.
During benchmarks, when reaching speeds of 2-3ms per round, the millisecond clock precision was insufficient.
I thus bumped it to microseconds for increased accuracy.
A bug also prevented from enabling profiling:
clCreateCommandQueueWithPropertieson OpenCL 2.0+ expects a zero terminated list of name/value pairs, not a bitfield like 1.2 –0just so happened to be interpreted as an empty list.Risk Assessment
Risk Level:
Risks & Impact
Fully backwards-compatible with no user-facing changes.
But changes how the kernels work in a non-trivial way.
Requires clearing the cache to see changes (and to rollback).
Disclaimer: this PR was assisted by Claude Opus 5 for code exploration & benchmark scripts.
However, all code being submitted has been hand-written – and I'm a human :)