diff --git a/cppwg/generators.py b/cppwg/generators.py index ee6a230..a198591 100644 --- a/cppwg/generators.py +++ b/cppwg/generators.py @@ -7,7 +7,6 @@ import shutil import subprocess import uuid -from pathlib import Path import pygccxml @@ -227,8 +226,10 @@ def log_unknown_classes(self) -> None: source_locations = self.package_info._module_source_locations() def in_source_locations(file_path: str) -> bool: - parents = Path(file_path).parents - return any(location in parents for location in source_locations) + return any( + utils.path_is_within(file_path, location) + for location in source_locations + ) seen_class_names = set() for module_info in self.package_info.module_collection: diff --git a/cppwg/info/base_info.py b/cppwg/info/base_info.py index 4f2bbeb..4fe152c 100644 --- a/cppwg/info/base_info.py +++ b/cppwg/info/base_info.py @@ -254,15 +254,25 @@ def hierarchy_attribute(self, attribute_name: str) -> Any: Any The attribute value, or None if not found. """ + # Memoize by attribute name: this walks the class -> module -> package + # chain on every call, and is only read during generation, by which point + # the gathered config is fixed. Lazily created so any BaseInfo subclass + # works whether or not it ran BaseInfo.__init__. + cache = self.__dict__.setdefault("_hierarchy_attribute_cache", {}) + if attribute_name in cache: + return cache[attribute_name] + value = getattr(self, attribute_name, None) if value or isinstance(value, bool) or isinstance(value, Number): - return value - - if self.parent is None: + result = value + elif self.parent is None: # Reached the top of the hierarchy (i.e. PackageInfo) - return None + result = None + else: + result = self.parent.hierarchy_attribute(attribute_name) - return self.parent.hierarchy_attribute(attribute_name) + cache[attribute_name] = result + return result def hierarchy_attribute_gather(self, attribute_name: str) -> list[Any]: """ @@ -319,10 +329,18 @@ def hierarchy_attribute_gather_flat(self, attribute_name: str) -> list[Any]: list[Any] The flattened list of items. """ + # Memoize by attribute name (see hierarchy_attribute). The cached list is + # returned directly; callers only read/concatenate it, never mutate it. + cache = self.__dict__.setdefault("_hierarchy_gather_flat_cache", {}) + if attribute_name in cache: + return cache[attribute_name] + flat: list[Any] = [] for value in self.hierarchy_attribute_gather(attribute_name): if isinstance(value, (list, tuple, set)): flat.extend(value) else: flat.append(value) + + cache[attribute_name] = flat return flat diff --git a/cppwg/info/module_info.py b/cppwg/info/module_info.py index 768348f..58770a0 100644 --- a/cppwg/info/module_info.py +++ b/cppwg/info/module_info.py @@ -1,12 +1,11 @@ """Module information structure.""" -from pathlib import Path from typing import TYPE_CHECKING, Any from pygccxml import declarations from cppwg.info.base_info import BaseInfo -from cppwg.info.class_info import CppClassInfo +from cppwg.info.class_info import CppClassInfo, _unqualified_base_name from cppwg.info.enum_info import CppEnumInfo from cppwg.info.free_function_info import CppFreeFunctionInfo from cppwg.utils import utils @@ -174,11 +173,10 @@ def is_decl_in_source_path(self, decl: "declaration_t") -> bool: if not self.source_locations: return True - for location in self.source_locations: - if Path(location) in Path(decl.location.file_name).parents: - return True - - return False + return any( + utils.path_is_within(decl.location.file_name, location) + for location in self.source_locations + ) def sort_classes(self) -> None: """ @@ -209,21 +207,58 @@ def sort_classes(self) -> None: } # Inheritance is a hard ordering constraint: a base precedes its - # subclasses. + # subclasses. Precompute each class's unqualified name and the set of + # unqualified names of the bases it declares, so the check is a set + # membership test (matching CppClassInfo.extends) rather than a per-pair + # scan over base_decls with a name reduction on each element. + unqualified_name = {cls: _unqualified_base_name(cls.name) for cls in classes} + declared_base_names = { + cls: { + _unqualified_base_name(base_decl.name) + for base_decl in cls.base_decls + if base_decl is not None + } + for cls in classes + } for cls in classes: + base_names = declared_base_names[cls] + if not base_names: + continue for other in classes: - if other is not cls and cls.extends(other): + if other is not cls and unqualified_name[other] in base_names: predecessors[cls].add(other) # Signature dependencies order a class after a wrapped type it uses, # unless that contradicts an inheritance ordering already recorded. - # Argument type strings are gathered once per class to keep this cheap. - arg_types = {cls: cls.signature_arg_types() for cls in classes} + # Precompute, per class, its argument-type strings in canonical form and + # a compiled whole-token regex for its name, so the dependency test does + # not re-canonicalize and re-compile on every one of the ~C^2 pair + # comparisons (previously the dominant CPU cost of a large generation). + canon_arg_types = { + cls: [ + utils.canonicalize_type_whitespace(arg_type) + for arg_type in cls.signature_arg_types() + ] + for cls in classes + } + name_regex = {cls: utils.compile_type_pattern(cls.name) for cls in classes} + requires_cache: dict[tuple[CppClassInfo, CppClassInfo], bool] = {} def requires(a: CppClassInfo, b: CppClassInfo) -> bool: - return any( - utils.type_string_matches(arg_type, b.name) for arg_type in arg_types[a] + # Whether any of a's public method/constructor argument types name b + # as a whole token. Equivalent to + # `any(utils.type_string_matches(t, b.name) for t in a's arg types)` + # but reusing the precomputed canonical arg types and compiled regex. + key = (a, b) + cached = requires_cache.get(key) + if cached is not None: + return cached + regex = name_regex[b] + result = regex is not None and any( + regex.search(arg_type) for arg_type in canon_arg_types[a] ) + requires_cache[key] = result + return result for cls in classes: for other in classes: diff --git a/cppwg/info/package_info.py b/cppwg/info/package_info.py index 7de61dd..8dd679b 100644 --- a/cppwg/info/package_info.py +++ b/cppwg/info/package_info.py @@ -5,7 +5,6 @@ import os import re from collections.abc import Iterator -from pathlib import Path from typing import TYPE_CHECKING, Any from pygccxml import declarations @@ -278,7 +277,7 @@ def collect_source_files( # Skip files in restricted paths if any( - Path(restricted_path) in Path(filepath).parents + utils.path_is_within(filepath, restricted_path) for restricted_path in restricted_paths ): continue @@ -358,7 +357,10 @@ def collect_source_cpp(self, restricted_paths: list[str]) -> None: cpp_files = [ filepath for filepath in cpp_files - if any(location in Path(filepath).parents for location in locations) + if any( + utils.path_is_within(filepath, location) + for location in locations + ) ] self.source_cpp_files = cpp_files @@ -760,7 +762,7 @@ def dependency(class_info: "CppClassInfo", decl) -> str | None: c for c in module_info.class_collection if c.cpp_names or c.excluded ] - def _module_source_locations(self) -> list[Path]: + def _module_source_locations(self) -> list[str]: """ Return the source-location paths that scope the wrapped source tree. @@ -773,15 +775,15 @@ def _module_source_locations(self) -> list[Path]: Returns ------- - list[pathlib.Path] + list[str] The directories that bound the project's own source files. """ - locations: list[Path] = [] + locations: list[str] = [] for module_info in self.module_collection: if module_info.source_locations: - locations.extend(Path(loc) for loc in module_info.source_locations) + locations.extend(module_info.source_locations) else: - locations.append(Path(self.source_root)) + locations.append(self.source_root) return locations def _build_type_header_map(self) -> dict[str, str]: @@ -807,8 +809,10 @@ def _build_type_header_map(self) -> dict[str, str]: source_locations = self._module_source_locations() def in_source_locations(file_path: str) -> bool: - parents = Path(file_path).parents - return any(location in parents for location in source_locations) + return any( + utils.path_is_within(file_path, location) + for location in source_locations + ) mapping: dict[str, str] = {} ambiguous: set[str] = set() diff --git a/cppwg/parsers/source_parser.py b/cppwg/parsers/source_parser.py index dd7d88a..69c4f47 100644 --- a/cppwg/parsers/source_parser.py +++ b/cppwg/parsers/source_parser.py @@ -2,7 +2,6 @@ import logging import os -from pathlib import Path from pygccxml import declarations, parser from pygccxml.declarations import declaration_t @@ -117,7 +116,7 @@ def parse(self) -> namespace_t: source_decls: list[declaration_t] = [ decl for decl in filtered_decls - if Path(self.source_root) in Path(decl.location.file_name).parents + if utils.path_is_within(decl.location.file_name, self.source_root) or decl.location.file_name == self.wrapper_header_collection ] @@ -181,13 +180,15 @@ class name to the template argument lists found. global_ns: namespace_t = declarations.get_global_namespace(decls) + # realpath(source_file) is loop-invariant; resolve it once rather + # than per declaration (each realpath is a filesystem syscall). + source_file_real = os.path.realpath(source_file) + for class_decl in global_ns.classes(allow_empty=True): # Keep only explicit instantiations defined in this file. if class_decl.location is None: continue - if os.path.realpath(class_decl.location.file_name) != os.path.realpath( - source_file - ): + if os.path.realpath(class_decl.location.file_name) != source_file_real: continue if not declarations.templates.is_instantiation(class_decl.name): diff --git a/cppwg/utils/utils.py b/cppwg/utils/utils.py index 75d329c..c68b8fb 100644 --- a/cppwg/utils/utils.py +++ b/cppwg/utils/utils.py @@ -244,18 +244,45 @@ def type_string_matches(type_string: str, pattern: str) -> bool: bool True if the pattern occurs in the type string as a whole token. """ + regex = compile_type_pattern(pattern) + if regex is None: + return False + + # Match on a whitespace-canonical form of the searched string so that + # differences in spacing around punctuation (which pygccxml and hand-written + # config may spell differently) do not defeat the match. + return regex.search(canonicalize_type_whitespace(type_string)) is not None + + +def compile_type_pattern(pattern: str) -> "re.Pattern | None": + """ + Compile a whole-token match regex for a C++ type pattern. + + Returns a compiled regex that matches ``pattern`` as a whole token in a + *whitespace-canonical* type string (see :func:`type_string_matches`), or + ``None`` if ``pattern`` is not a usable pattern (not a non-empty string, or + empty once canonicalized). Splitting the compile out lets a caller that + tests one pattern against many strings (e.g. class-dependency sorting) + canonicalize and compile the pattern once instead of on every comparison. + + Parameters + ---------- + pattern : str + The type pattern to look for. + + Returns + ------- + re.Pattern | None + The compiled whole-token regex, or None if the pattern is unusable. + """ # A non-string pattern (e.g. a yaml scalar like `arg_type_excludes: 5`) is # not a valid type pattern; treat it as non-matching rather than crashing. if not isinstance(pattern, str) or not pattern: - return False + return None - # Match on a whitespace-canonical form of both strings so that differences - # in spacing around punctuation (which pygccxml and hand-written config may - # spell differently) do not defeat the match. - type_string = canonicalize_type_whitespace(type_string) pattern = canonicalize_type_whitespace(pattern) if not pattern: - return False + return None # Enforce an identifier boundary only on an edge whose pattern character is # itself an identifier character. A pattern ending in e.g. > / * / & should @@ -264,8 +291,37 @@ def type_string_matches(type_string: str, pattern: str) -> bool: left = r"(? bool: + """ + Return whether ``path`` lies strictly beneath the directory ``ancestor``. + + Equivalent to ``Path(ancestor) in Path(path).parents`` but implemented with + normalized-string comparison rather than allocating ``Path`` objects and + scanning the ``parents`` sequence. The ``Path``-based form dominated + profiled generation time (millions of per-declaration and per-file checks), + so this cheaper equivalent is used on the hot paths. + + Like ``Path.parents``, the test is *lexical* (no symlink resolution) and + *strict*: a path equal to ``ancestor`` is not "within" it. + + Parameters + ---------- + path : str + The candidate descendant path. + ancestor : str + The directory that ``path`` may live beneath. + + Returns + ------- + bool + True if ``path`` is strictly beneath ``ancestor``. + """ + ancestor = os.path.normpath(ancestor) + path = os.path.normpath(path) + return path.startswith(ancestor + os.sep) def type_is_copy_assignable(decl_type: Any) -> bool: diff --git a/cppwg/writers/class_writer.py b/cppwg/writers/class_writer.py index 51ced1e..3aeff70 100644 --- a/cppwg/writers/class_writer.py +++ b/cppwg/writers/class_writer.py @@ -37,6 +37,76 @@ from cppwg.info.class_info import CppClassInfo +def virtual_method_signature(method_decl: "member_function_t") -> tuple: + """ + Return the identity used to match an override against a base virtual. + + The tuple is (name, const-ness, argument-type strings). The return type is + intentionally excluded so a covariant-return override still matches, and each + argument type has its whitespace canonicalized so spelling differences between + the derived and base declarations do not defeat the match. + """ + return ( + method_decl.name, + method_decl.has_const, + tuple( + canonicalize_type_whitespace(t.decl_string) + for t in method_decl.argument_types + ), + ) + + +def build_base_virtual_signature_index( + package_classes: set["class_t"], + package_class_infos: dict["class_t", "CppClassInfo"], +) -> dict["class_t", set]: + """ + Precompute, per wrapped class, the virtual signatures it actually binds. + + For every class wrapped in the package, collect the virtual_method_signature + of each public virtual member function it binds (i.e. not dropped by + CppMethodWrapperWriter.method_is_excluded). With this index, + _overrides_wrapped_base_virtual reduces to a set-membership test against a + base's entry, instead of re-querying and re-comparing the base's member + functions for every override of every derived class - the same work was + previously repeated for each candidate method. Built once and shared by all + class writers. + + Parameters + ---------- + package_classes : set[pygccxml.declarations.class_t] + Declarations of every class wrapped anywhere in the package. + package_class_infos : dict[class_t, CppClassInfo] + Maps each such decl to its class_info, needed for method_is_excluded. + + Returns + ------- + dict[class_t, set[tuple]] + Maps each wrapped class decl to the set of virtual signatures it binds. + """ + index: dict["class_t", set] = {} + for base_decl in package_classes: + base_info = package_class_infos.get(base_decl) + signatures: set = set() + for base_method in base_decl.member_functions(allow_empty=True): + # Only public methods are bound (build_class_register filters on public + # access), so a protected/private base virtual is not wrapped on the + # base and cannot make an override redundant. + if base_method.access_type != "public": + continue + if base_method.virtuality not in ("virtual", "pure virtual"): + continue + # A base method excluded from wrapping (by name, return type or arg + # type) emits no binding, so it cannot make an override redundant. + if base_info is not None and CppMethodWrapperWriter.method_is_excluded( + base_info, base_decl, base_method + ): + continue + signatures.add(virtual_method_signature(base_method)) + index[base_decl] = signatures + return index + + class CppClassWrapperWriter(CppBaseWrapperWriter): """ Writer to generate wrapper code for C++ classes. @@ -71,6 +141,7 @@ def __init__( package_classes: set["class_t"] = None, overwrite: bool = False, package_class_infos: dict["class_t", "CppClassInfo"] = None, + base_virtual_signatures: dict["class_t", set] = None, ) -> None: logger = logging.getLogger() @@ -87,6 +158,12 @@ def __init__( self.package_class_infos = ( package_class_infos if package_class_infos is not None else {} ) + # Prebuilt per-package index of each base's bound virtual signatures, + # consulted by _overrides_wrapped_base_virtual. Normally supplied by the + # module writer so every class writer shares one build; if a caller omits + # it (e.g. a unit test), it is built lazily from this writer's package + # classes on first use (see the base_virtual_signatures property). + self._base_virtual_signatures = base_virtual_signatures self.overwrite = overwrite @@ -97,6 +174,25 @@ def __init__( # from the generated registration text in write(). Empty until then. self.typecaster_includes: list[str] = [] + # Memoization for the inherited-override test, which runs for every method + # (and every sibling overload) of every class this writer emits. The + # linked-base list is identical for all methods of a class_decl, and a + # method's signature is recomputed for each sibling scan, so both are + # cached rather than recomputed per call. See _overrides_wrapped_base_virtual. + self._linked_wrapped_bases_cache: dict["class_t", list["class_t"]] = {} + self._method_signature_cache: dict["member_function_t", tuple] = {} + + @property + def base_virtual_signatures(self) -> dict["class_t", set]: + """ + Per-base bound-virtual signature index, built lazily if not supplied. + """ + if self._base_virtual_signatures is None: + self._base_virtual_signatures = build_base_virtual_signature_index( + self.package_classes, self.package_class_infos + ) + return self._base_virtual_signatures + def prefix_block(self) -> str: """ Return the prefix text block for the top of a wrapper file. @@ -470,6 +566,10 @@ def _overrides_wrapped_base_virtual( test used by _is_inherited_override; it does not consider sibling overloads. + The wrapped-virtual match is a set lookup against + base_virtual_signatures (see build_base_virtual_signature_index), which + precomputes each base's bound virtual signatures once for the whole package. + Parameters ---------- class_decl : pygccxml.declarations.class_t @@ -485,18 +585,38 @@ def _overrides_wrapped_base_virtual( if method_decl.virtuality not in ("virtual", "pure virtual"): return False - # Cross-module inheritance is only linked into the derived py::class_ when - # the module opts in via `imports` (see bases_block). Without it, a base - # wrapped in another module contributes no inherited binding, so an - # override of it here is the sole binding and must not be skipped. - allow_external_bases = bool(self.class_info.hierarchy_attribute("imports")) + signature = self._method_signature_cache.get(method_decl) + if signature is None: + signature = virtual_method_signature(method_decl) + self._method_signature_cache[method_decl] = signature - name = method_decl.name - arg_types = [ - canonicalize_type_whitespace(t.decl_string) - for t in method_decl.argument_types - ] + for base_decl in self._linked_wrapped_bases(class_decl): + if signature in self.base_virtual_signatures.get(base_decl, ()): + return True + + return False + + def _linked_wrapped_bases(self, class_decl: "class_t") -> list["class_t"]: + """ + Return the wrapped bases whose bindings this class actually inherits. + + A base qualifies if it is wrapped in this package and its pybind base link + is emitted into the derived py::class_: a same-module base is always + linked, but a base wrapped in another module is linked only when + cross-module inheritance is enabled (`imports` set) - see bases_block. + Without that link the base's binding is not inherited, so an override of it + would become unreachable if skipped. + + The result is identical for every method of ``class_decl`` and is cached, + so recursive_bases is walked once per class rather than once per method. + """ + cached = self._linked_wrapped_bases_cache.get(class_decl) + if cached is not None: + return cached + allow_external_bases = bool(self.class_info.hierarchy_attribute("imports")) + + bases: list["class_t"] = [] for hierarchy_info in class_decl.recursive_bases: base_decl = hierarchy_info.related_class # Skip bases pygccxml could not resolve, and bases not wrapped in this @@ -508,36 +628,10 @@ def _overrides_wrapped_base_virtual( # base (in module_classes) is always linked. if base_decl not in self.module_classes and not allow_external_bases: continue + bases.append(base_decl) - for base_method in base_decl.member_functions(name, allow_empty=True): - # Only public methods are bound (build_class_register filters on - # public access), so a protected/private base virtual is not - # wrapped on the base and cannot make this override redundant. - if base_method.access_type != "public": - continue - if base_method.virtuality not in ("virtual", "pure virtual"): - continue - if base_method.has_const != method_decl.has_const: - continue - base_arg_types = [ - canonicalize_type_whitespace(t.decl_string) - for t in base_method.argument_types - ] - if base_arg_types != arg_types: - continue - # The base declares a matching virtual. Keep the override only if - # the base does not actually wrap it: a base method excluded from - # wrapping (by name, return type or arg type - the same rules as - # CppMethodWrapperWriter) emits no binding, so this override is the - # sole binding and must not be skipped. - base_info = self.package_class_infos.get(base_decl) - if base_info is not None and CppMethodWrapperWriter.method_is_excluded( - base_info, base_decl, base_method - ): - continue - return True - - return False + self._linked_wrapped_bases_cache[class_decl] = bases + return bases def _is_inherited_override( self, class_decl: "class_t", method_decl: "member_function_t" diff --git a/cppwg/writers/module_writer.py b/cppwg/writers/module_writer.py index e379df8..69b04e0 100644 --- a/cppwg/writers/module_writer.py +++ b/cppwg/writers/module_writer.py @@ -11,7 +11,10 @@ registration_function_name, write_file_if_changed, ) -from cppwg.writers.class_writer import CppClassWrapperWriter +from cppwg.writers.class_writer import ( + CppClassWrapperWriter, + build_base_virtual_signature_index, +) from cppwg.writers.enum_writer import CppEnumWrapperWriter from cppwg.writers.free_function_writer import CppFreeFunctionWrapperWriter @@ -294,6 +297,19 @@ def write_class_wrappers(self) -> None: """Write wrappers for classes in the module.""" logger = logging.getLogger() + # Index of each base class's bound virtual signatures, consulted when + # skipping redundant inherited overrides. It is package-wide (identical for + # every module) and its build scans every wrapped class's member functions, + # so build it once and cache it on the package info, then share it across + # all class writers rather than have each rebuild it. + pkg_info = self.module_info.package_info + base_virtual_signatures = getattr(pkg_info, "_base_virtual_signatures", None) + if base_virtual_signatures is None: + base_virtual_signatures = build_base_virtual_signature_index( + self.package_classes, self.package_class_infos + ) + pkg_info._base_virtual_signatures = base_virtual_signatures + seen_file_stems: dict[str, str] = {} for class_info in self.module_info.class_collection: # Skip excluded classes @@ -325,6 +341,7 @@ def write_class_wrappers(self) -> None: self.package_classes, self.overwrite, self.package_class_infos, + base_virtual_signatures, ) # Write the class wrappers into /path/to/wrapper_root/modulename/