Skip to content

Commit 4319088

Browse files
rustyconoverclaude
andcommitted
perf: find abstract attributes in one MRO walk at subclass creation
TableFunctionBase.__init_subclass__ called inspect.getattr_static for every name in dir(cls), and each call walks the MRO again. Walk the MRO once over each class's __dict__ instead: the same raw, descriptor-free attributes, about 20x cheaper. A worker that defines thousands of function classes at startup (vgi-aws: ~8,000) spends noticeably less time building them: 3.3s -> 2.3s boot on macOS, 9.4s -> 7.0s on a Graviton Linux host. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 0b993d2 commit 4319088

1 file changed

Lines changed: 34 additions & 15 deletions

File tree

‎vgi/table_function.py‎

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
from __future__ import annotations
1010

11-
import inspect
1211
import uuid
1312
from abc import abstractmethod
1413
from collections.abc import Mapping
@@ -197,6 +196,38 @@ class TableCardinality(ArrowSerializableDataclass):
197196
max: int | None
198197

199198

199+
def _has_abstract_attribute(cls: type) -> bool:
200+
"""Whether any attribute of ``cls`` is still abstract.
201+
202+
``ABCMeta`` only fills in ``__abstractmethods__`` after ``__init_subclass__``
203+
returns, so a class under construction has to be inspected by hand.
204+
205+
Each attribute is read raw, from the ``__dict__`` of the first class in the MRO
206+
that defines it, without invoking descriptors. A plain ``getattr()`` resolved
207+
the inherited ``Function.storage`` descriptor, whose first access builds the
208+
default SQLite store on disk. Because vgi defines subclasses of its own, that
209+
made ``import vgi`` alone create a database under the user's state directory,
210+
and fail outright where that directory is not writable (a container's non-root
211+
user, say). The raw attribute answers the same question: classmethod,
212+
staticmethod and property objects all expose the ``__isabstractmethod__`` of
213+
what they wrap.
214+
215+
This used to call ``inspect.getattr_static`` for every name in ``dir(cls)``.
216+
That walks the MRO once per name; one walk for all names gives the same answer
217+
and is ~20x cheaper, which matters to a worker that defines thousands of
218+
function classes at startup.
219+
"""
220+
seen: set[str] = set()
221+
for klass in cls.__mro__:
222+
for name, value in vars(klass).items():
223+
if name in seen:
224+
continue
225+
seen.add(name)
226+
if getattr(value, "__isabstractmethod__", False):
227+
return True
228+
return False
229+
230+
200231
def _batch_to_scalar_dict(batch: pa.RecordBatch | None) -> dict[str, pa.Scalar[Any]]:
201232
"""Extract a single-row RecordBatch into a dict of column-name to scalar value.
202233
@@ -839,20 +870,8 @@ def __init_subclass__(cls) -> None:
839870
cls.FunctionArguments = type_args[0]
840871
break
841872

842-
# Skip validation for abstract base classes. Each attribute is read with
843-
# getattr_static, which returns the raw class attribute without invoking
844-
# descriptors: a plain getattr() here resolved the inherited
845-
# `Function.storage` descriptor, whose first access builds the default
846-
# SQLite store on disk. Because vgi defines subclasses of its own, that
847-
# made `import vgi` alone create a database under the user's state
848-
# directory, and fail outright where that directory is not writable (a
849-
# container's non-root user, say). The raw attribute answers the same
850-
# question: classmethod, staticmethod and property objects all expose
851-
# the `__isabstractmethod__` of what they wrap.
852-
is_abstract = any(
853-
getattr(inspect.getattr_static(cls, name, None), "__isabstractmethod__", False) for name in dir(cls)
854-
)
855-
if is_abstract:
873+
# Skip validation for abstract base classes.
874+
if _has_abstract_attribute(cls):
856875
cls._setting_params = {}
857876
cls._secret_params = {}
858877
return

0 commit comments

Comments
 (0)