Speed up vkGet{Instance,Device}ProcAddr lookups

loader/generated/vk_loader_extensions.h

@@ -45,7 +45,9 @@ VKAPI_ATTR VkResult VKAPI_CALL vkDevExtError(VkDevice dev);
 
 // Extension interception for vkGetInstanceProcAddr function, so we can return
 // the appropriate information for any instance extensions we know about.
-bool extension_instance_gpa(struct loader_instance *ptr_instance, const char *name, void **addr);
+// name_hash must be loader_hash_string(name) - callers that already have it (e.g.
+// trampoline_get_proc_addr) pass it through instead of hashing name a second time.
+bool extension_instance_gpa(struct loader_instance *ptr_instance, const char *name, uint32_t name_hash, void **addr);
 
 struct loader_instance_extension_enable_list; // Forward declaration
 

loader/loader_common.h

@@ -59,6 +59,27 @@ static const unsigned char UTF8_THREE_BYTE_MASK = 248;  // 0xF8;
 static const unsigned char UTF8_DATA_BYTE_CODE = 128;   // 0x80;
 static const unsigned char UTF8_DATA_BYTE_MASK = 192;   // 0xC0;
 
+// 32-bit FNV-1a (https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function). Mirrored
+// byte-for-byte in Python as loader_hash_string() in scripts/generators/loader_extension_generator.py,
+// which precomputes the hash constants embedded in loader/generated/vk_loader_extensions.c - keep both in
+// sync or those constants stop matching this function (see HashString.MatchesGeneratorEmbeddedConstants in
+// tests/loader_hash_string_tests.cpp).
+//
+// Used as a cheap pre-filter before the strcmp confirmation when resolving entry point names
+// (vkGet{Instance,Device}ProcAddr): a hash match still falls through to a switch/case-guarded strcmp before
+// anything is returned, so a collision can never produce a wrong lookup - at worst it costs one extra
+// strcmp. Bounded by MaxLoaderStringLength so a non-NUL-terminated or adversarially long input can't make
+// this scan past a bounded byte count the way an unbounded `while (*str)` would; every real entry point
+// name is far shorter than the cap, so this never changes the hash of a valid name.
+static inline uint32_t loader_hash_string(const char *str) {
+    uint32_t hash = 2166136261u;
+    for (uint32_t i = 0; i < (uint32_t)MaxLoaderStringLength && str[i]; ++i) {
+        hash ^= (uint8_t)str[i];
+        hash *= 16777619u;
+    }
+    return hash;
+}
+
 // form of all dynamic lists/arrays
 // only the list element should be changed
 struct loader_generic_list {

scripts/generators/loader_extension_generator.py

@@ -104,6 +104,57 @@
                           'vkEnumerateInstanceLayerProperties',
                           'vkEnumerateInstanceVersion']
 
+def _read_max_loader_string_length_from_c_header() -> int:
+    header_path = os.path.join(os.path.dirname(__file__), '..', '..', 'loader', 'loader_common.h')
+    with open(header_path, 'r', encoding='utf-8') as f:
+        header_text = f.read()
+    match = re.search(r'^\s*static const int MaxLoaderStringLength\s*=\s*(\d+)\s*;', header_text, re.MULTILINE)
+    assert match, (
+        f'Could not find "static const int MaxLoaderStringLength = <N>;" in {header_path}. '
+        'loader_hash_string() below needs this bound to stay in sync with loader/loader_common.h - '
+        'did that declaration get renamed or reformatted?')
+    return int(match.group(1))
+
+# Must match MaxLoaderStringLength in loader/loader_common.h - loader_hash_string() there stops scanning
+# after this many bytes (a name of exactly this length is scanned in full and hashes the same either way;
+# only a name LONGER than this would hash differently in C than in Python below). Parsed directly out of
+# the header (rather than hand-copied) so the two can never silently drift apart.
+MAX_LOADER_STRING_LENGTH = _read_max_loader_string_length_from_c_header()
+
+# 32-bit FNV-1a (https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function). Must stay
+# byte-for-byte identical to loader_hash_string() in loader/loader_common.h - this precomputes the hash
+# constants baked into the generated *_gpa()/dispatch-table-lookup functions below, which the C function
+# then has to reproduce at runtime to hit the right branch. HashString.MatchesGeneratorEmbeddedConstants in
+# tests/loader_hash_string_tests.cpp catches any divergence.
+def loader_hash_string(s: str) -> int:
+    assert len(s) <= MAX_LOADER_STRING_LENGTH, (
+        f'"{s}" is {len(s)} bytes, over MaxLoaderStringLength ({MAX_LOADER_STRING_LENGTH}). '
+        'loader_hash_string() in loader/loader_common.h truncates its scan at that length, so this name '
+        "would hash differently at runtime than the constant this generator is about to embed for it - "
+        'shorten the name or raise MaxLoaderStringLength in both the C and Python copies.')
+    h = 0x811c9dc5
+    for c in s.encode('ascii'):
+        h ^= c
+        h = (h * 0x01000193) & 0xFFFFFFFF
+    return h
+
+# Raises if any two names in `names` share a hash - used as a generation-time tripwire so that if the
+# Vulkan command set ever grows into a collision, it's caught immediately with a clear message instead of
+# failing to compile with an inscrutable "duplicate case value" error naming neither colliding string (each
+# name in `names` becomes its own `case <hash>u:` label in the generated switch - see OutputLoaderLookupFunc
+# and InstExtensionGPA below).
+def check_no_hash_collisions(names, context: str):
+    seen = {}
+    for name in names:
+        h = loader_hash_string(name)
+        if h in seen and seen[h] != name:
+            raise RuntimeError(
+                f'FNV-1a hash collision in {context}: "{seen[h]}" and "{name}" both hash to {h:#010x}. '
+                'Left unaddressed, this would make the generated switch emit two identical case labels and '
+                'fail to compile with a "duplicate case value" error naming neither string - '
+                'loader_hash_string() (C and Python copies) should be revisited (e.g. a different seed/prime).')
+        seen[h] = name
+
 # This class is a container for any source code, data, or other behavior that is necessary to
 # customize the generator script for a specific target API variant (e.g. Vulkan SC). As such,
 # all of these API-specific interfaces and their use in the generator script are part of the
@@ -275,7 +326,9 @@ def OutputPrototypesInHeader(self, out: list):
 
 // Extension interception for vkGetInstanceProcAddr function, so we can return
 // the appropriate information for any instance extensions we know about.
-bool extension_instance_gpa(struct loader_instance *ptr_instance, const char *name, void **addr);
+// name_hash must be loader_hash_string(name) - callers that already have it (e.g.
+// trampoline_get_proc_addr) pass it through instead of hashing name a second time.
+bool extension_instance_gpa(struct loader_instance *ptr_instance, const char *name, uint32_t name_hash, void **addr);
 
 struct loader_instance_extension_enable_list; // Forward declaration
 
@@ -704,6 +757,7 @@ def OutputLoaderLookupFunc(self, out):
                 out.append('    struct loader_device* dev = (struct loader_device *)table;\n')
                 out.append('    const struct loader_instance* inst = dev->phys_dev_term->this_icd_term->this_instance;\n')
                 out.append('    uint32_t api_version = VK_MAKE_API_VERSION(0, inst->app_api_version.major, inst->app_api_version.minor, inst->app_api_version.patch);\n')
+                out.append('    const uint32_t name_hash = loader_hash_string(name);\n')
                 out.append('\n')
             else:
                 cur_type = 'instance'
@@ -718,8 +772,11 @@ def OutputLoaderLookupFunc(self, out):
                 out.append('\n')
                 out.append('    *found_name = true;\n')
                 out.append('    name += 2;\n')
+                out.append('    const uint32_t name_hash = loader_hash_string(name);\n')
 
+            out.append('    switch (name_hash) {\n')
 
+            base_names_for_collision_check = []
             for command_list in [self.core_commands, self.extension_commands]:
                 commands = command_list
 
@@ -729,12 +786,12 @@ def OutputLoaderLookupFunc(self, out):
                     is_inst_handle_type = command.params[0].type in ['VkInstance', 'VkPhysicalDevice']
                     if ((cur_type == 'instance' and is_inst_handle_type) or (cur_type == 'device' and not is_inst_handle_type)):
 
-                        current_block = self.DescribeBlock(command, current_block, out)
+                        current_block = self.DescribeBlock(command, current_block, out, indent='        ')
                         if len(command.extensions) == 0:
                             if cur_type == 'device':
                                 effective_version_name = APISpecific.getEffectiveVersionName(self.targetApiName, command.version)
                                 api_version = effective_version_name.replace('_VERSION_', '_API_VERSION_')
-                                version_check = f"        if (dev->should_ignore_device_commands_from_newer_version && api_version < {api_version}) return NULL;\n"
+                                version_check = f"                if (dev->should_ignore_device_commands_from_newer_version && api_version < {api_version}) return NULL;\n"
                             else:
                                 version_check = ''
 
@@ -750,22 +807,29 @@ def OutputLoaderLookupFunc(self, out):
                         if command.protect is not None:
                             out.append(f'#if defined({command.protect})\n')
 
-                        out.append(f'    if (!strcmp(name, "{base_name}")) ')
+                        base_names_for_collision_check.append(base_name)
+                        out.append(f'        case {loader_hash_string(base_name):#010x}u:\n')
+                        out.append(f'            if (!strcmp(name, "{base_name}")) ')
                         if command.name in DEVICE_CMDS_MUST_USE_TRAMP:
                             if version_check != '':
-                                out.append(f'{{\n{version_check}        return dev->layer_extensions.{command.extensions[0][3:].lower()}_enabled ? (void *){base_name} : NULL;\n    }}\n')
+                                out.append(f'{{\n{version_check}                return dev->layer_extensions.{command.extensions[0][3:].lower()}_enabled ? (void *){base_name} : NULL;\n            }}\n')
                             else:
                                 out.append(f'return dev->layer_extensions.{command.extensions[0][3:].lower()}_enabled ? (void *){base_name} : NULL;\n')
 
                         else:
                             if version_check != '':
-                                out.append(f'{{\n{version_check}        return (void *)table->{base_name};\n    }}\n')
+                                out.append(f'{{\n{version_check}                return (void *)table->{base_name};\n            }}\n')
                             else:
                                 out.append(f'return (void *)table->{base_name};\n')
+                        out.append('            break;\n')
 
                         if command.protect is not None:
                             out.append(f'#endif // {command.protect}\n')
 
+            check_no_hash_collisions(base_names_for_collision_check, f'loader_lookup_{cur_type}_dispatch_table')
+            out.append('        default:\n')
+            out.append('            break;\n')
+            out.append('    }\n')
             out.append('\n')
             out.append('    *found_name = false;\n')
             out.append('    return NULL;\n')
@@ -1187,9 +1251,12 @@ def InstExtensionGPA(self, out):
         cur_extension_name = ''
 
         out.append( '// GPA helpers for extensions\n')
-        out.append( 'bool extension_instance_gpa(struct loader_instance *ptr_instance, const char *name, void **addr) {\n')
-        out.append( '    *addr = NULL;\n\n')
+        out.append( '// name_hash must be loader_hash_string(name) - passed in by the caller so it is only computed once per lookup.\n')
+        out.append( 'bool extension_instance_gpa(struct loader_instance *ptr_instance, const char *name, uint32_t name_hash, void **addr) {\n')
+        out.append( '    *addr = NULL;\n')
+        out.append( '    switch (name_hash) {\n')
 
+        full_names_for_collision_check = []
         for command in [x for x in self.vk.commands.values() if x.extensions]:
             if (command.version or
                 command.extensions[0] in WSI_EXT_NAMES or
@@ -1198,7 +1265,7 @@ def InstExtensionGPA(self, out):
                 continue
 
             if command.extensions[0] != cur_extension_name:
-                out.append( f'\n    // ---- {command.extensions[0]} extension commands\n')
+                out.append( f'\n        // ---- {command.extensions[0]} extension commands\n')
                 cur_extension_name = command.extensions[0]
 
             if command.protect is not None:
@@ -1207,24 +1274,31 @@ def InstExtensionGPA(self, out):
             #base_name = command.name[2:]
             base_name = SHARED_ALIASES[command.name] if command.name in SHARED_ALIASES else command.name[2:]
 
+            full_names_for_collision_check.append(command.name)
+            out.append(f'        case {loader_hash_string(command.name):#010x}u:\n')
             if len(command.extensions) > 0 and self.vk.extensions[command.extensions[0]].instance:
-                out.append( f'    if (!strcmp("{command.name}", name)) {{\n')
-                out.append( '        *addr = (ptr_instance->enabled_extensions.')
+                out.append( f'            if (!strcmp("{command.name}", name)) {{\n')
+                out.append( '                *addr = (ptr_instance->enabled_extensions.')
                 out.append( command.extensions[0][3:].lower())
                 out.append( ' == 1)\n')
-                out.append( f'                     ? (void *){base_name}\n')
-                out.append( '                     : NULL;\n')
-                out.append( '        return true;\n')
-                out.append( '    }\n')
+                out.append( f'                             ? (void *){base_name}\n')
+                out.append( '                             : NULL;\n')
+                out.append( '                return true;\n')
+                out.append( '            }\n')
             else:
-                out.append( f'    if (!strcmp("{command.name}", name)) {{\n')
-                out.append( f'        *addr = (void *){base_name};\n')
-                out.append( '        return true;\n')
-                out.append( '    }\n')
+                out.append( f'            if (!strcmp("{command.name}", name)) {{\n')
+                out.append( f'                *addr = (void *){base_name};\n')
+                out.append( '                return true;\n')
+                out.append( '            }\n')
+            out.append('            break;\n')
 
             if command.protect is not None:
                 out.append( f'#endif // {command.protect}\n')
 
+        check_no_hash_collisions(full_names_for_collision_check, 'extension_instance_gpa')
+        out.append( '        default:\n')
+        out.append( '            break;\n')
+        out.append( '    }\n')
         out.append( '    return false;\n')
         out.append( '}\n\n')
 

tests/CMakeLists.txt

@@ -81,6 +81,7 @@ add_executable(
         loader_alloc_callback_tests.cpp
         loader_envvar_tests.cpp
         loader_get_proc_addr_tests.cpp
+        loader_hash_string_tests.cpp
         loader_debug_ext_tests.cpp
         loader_handle_validation_tests.cpp
         loader_layer_tests.cpp
@@ -92,6 +93,7 @@ add_executable(
         loader_wsi_tests.cpp)
 target_link_libraries(test_regression PUBLIC testing_dependencies)
 target_compile_definitions(test_regression PUBLIC VK_NO_PROTOTYPES)
+target_include_directories(test_regression PRIVATE ${CMAKE_CURRENT_LIST_DIR}/../loader ${CMAKE_CURRENT_LIST_DIR}/../loader/generated)
 
 add_executable(
     test_fuzzing