Speed up vkGet{Instance,Device}ProcAddr lookups - #1991
Conversation
|
Author jpr42 not on autobuild list. Waiting for curator authorization before starting CI build. |
1 similar comment
|
Author jpr42 not on autobuild list. Waiting for curator authorization before starting CI build. |
|
CI passes on fork. CI failure is unrelated/sporadic. Try running failed jobs again. |
066ed53 to
52f1299
Compare
charles-lunarg
left a comment
charles-lunarg
left a comment
There was a problem hiding this comment.
Ran the tests 5 times and measured the difference.
Its not significant, probably because the filesystem & library loading dwarf the test runtime. Also this is with address sanitizer enabled, so I'm sure that is a large portion of the runtime. This change
main
8.21s, 8.37s, 8.19s, 8.34s, 8.33s
avg 8.29s
dev-hash-1631
7.9s, 8.28s, 7.97s, 8.34s, 8.56s
avg 8.21s
A "real" test would be running validation layer tests, since they create/destroy instances all the time without loading libraries or dealing with filesystems.
| // exactly so a mistake made while mechanically threading the hash pre-filter through ~230 hand-written | ||
| // strcmp branches (wrong hash constant, dropped branch, mismatched name) shows up as a concrete failure | ||
| // here instead of silently returning NULL/wrong-pointer for some entry point. | ||
| static constexpr std::array kGpaHelperCoreInstanceNames = { |
There was a problem hiding this comment.
As much as I want to require that this be code-generated, the fact that there are hand written dispatchers makes that hypocritical. So a follow up PR to make this be code generated along with generating gpa_helper.c makes more sense.
|
So your PR isn't being run through our internal CI, and I think it is because the repo name doesn't match. I'm not going to ask you to change it (thats a pretty silly requirement just to have a few extra test runs). Rather I'll do a manual run with your fork as the base to ensure nothing is broken. And as expected, it passed without issue. |
Benchmarked against the pre-change baseline: typical and late-position lookups are 20-70x faster, and unknown-name (miss) lookups - the most common real-world case when probing for unsupported extensions - are 10-70x faster. The only downside is a small, fixed regression (a few ns, still under 20ns) for the single name that happened to be first in each function's original if-chain, due to the fixed cost of hashing the query string. Fixes KhronosGroup#1631
charles-lunarg
left a comment
charles-lunarg
left a comment
There was a problem hiding this comment.
Much appreciate you taking this on and making it a reality including making it work with codegen and with tests that verify the codegen/python implementation are identical.
3 participants
Benchmarked against the pre-change baseline: typical and late-position lookups are 20-70x faster, and unknown-name (miss) lookups - the most common real-world case when probing for unsupported extensions - are 10-70x faster. The only downside is a small, fixed regression (a few ns, still under 20ns) for the single name that happened to be first in each function's original if-chain, due to the fixed cost of hashing the query string.
Fixes #1631