From e6dd732e3fa88f334f017c9c8136a693490b4247 Mon Sep 17 00:00:00 2001 From: samschott Date: Tue, 3 Dec 2024 21:24:47 +0100 Subject: [PATCH 1/6] clean up method loading recursion --- src/rubicon/objc/api.py | 101 +++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 54 deletions(-) diff --git a/src/rubicon/objc/api.py b/src/rubicon/objc/api.py index a4349cfa..cb8afe0b 100644 --- a/src/rubicon/objc/api.py +++ b/src/rubicon/objc/api.py @@ -1057,22 +1057,17 @@ def __getattr__(self, name): if method: return ObjCBoundMethod(method, self)() + # Load the class's methods if we haven't done so yet. + with self.objc_class.cache_lock: + self.objc_class._load_methods() + + method = None # See if there's a partial method starting with the given name, # either on self's class or any of the superclasses. - cls = self.objc_class - while cls is not None: - # Load the class's methods if we haven't done so yet. - with cls.cache_lock: - if cls.methods_ptr is None: - cls._load_methods() - - try: - method = cls.partial_methods[name] - break - except KeyError: - cls = cls.superclass - else: - method = None + try: + method = self.objc_class.partial_methods[name] + except KeyError: + pass if method is None or set(method.methods) == {()}: # Find a method whose full name matches the given name if no partial @@ -1430,36 +1425,36 @@ def _cache_method(self, name): by looking it up in the cached list of methods or by searching for and creating a new method object.""" with self.cache_lock: + # Try to return an existing cached method for the name try: - # Try to return an existing cached method for the name return self.instance_methods[name] except KeyError: - supercls = self - objc_method = None - while supercls is not None: - # Load the class's methods if we haven't done so yet. - if supercls.methods_ptr is None: - supercls._load_methods() - - try: - objc_method = supercls.instance_methods[name] - break - except KeyError: - pass + pass - try: - objc_method = ObjCMethod(supercls.instance_method_ptrs[name]) + # Load the class's methods if we haven't done so yet. + if self.methods_ptr is None: + self._load_methods() + + # Try to find a cached ObjCMethod method. Those are stored directly with us. + objc_method = self.instance_methods.get(name) + + # Try to find a method pointer in the class hierarchy. Those are stored + # with each superclass. + if objc_method is None: + cls = self + while cls is not None: + objc_method_ptr = cls.instance_method_ptrs.get(name) + if objc_method_ptr is not None: + objc_method = ObjCMethod(objc_method_ptr) break - except KeyError: - pass - supercls = supercls.superclass + cls = cls.superclass + + if objc_method is None: + return None - if objc_method is None: - return None - else: - self.instance_methods[name] = objc_method - return objc_method + self.instance_methods[name] = objc_method + return objc_method def _cache_property_methods(self, name): """Return the accessor and mutator for the named property.""" @@ -1617,24 +1612,22 @@ def __subclasscheck__(self, subclass): ) def _load_methods(self): - if self.methods_ptr is not None: - raise RuntimeError(f"{self}._load_methods cannot be called more than once") - - # Traverse superclasses and load methods. - superclass = self.superclass - - while superclass is not None: - if superclass.methods_ptr is None: - with superclass.cache_lock: - superclass._load_methods() - - # Prime this class' partials list with a list from the superclass. - for first, superpartial in superclass.partial_methods.items(): - partial = ObjCPartialMethod(first) - self.partial_methods[first] = partial - partial.methods.update(superpartial.methods) + # Traverse superclasses and load methods. Always do this, even if _load_methods + # was called previously, because the class hierarchy may have changed. + if self.superclass is not None: + with self.superclass.cache_lock: + self.superclass._load_methods() + + # Update this class' partials list with a list from the superclass. + for first, superpartial in self.superclass.partial_methods.items(): + try: + partial = self.partial_methods[first] + partial.methods.update(superpartial.methods) + except KeyError: + self.partial_methods[first] = superpartial - superclass = superclass.superclass + if self.methods_ptr is not None: + return # Load methods for this class. methods_ptr_count = c_uint(0) From 8773b6de42bd46582ecb401bc7353da7a1a3fb72 Mon Sep 17 00:00:00 2001 From: samschott Date: Tue, 3 Dec 2024 21:48:50 +0100 Subject: [PATCH 2/6] update inline docs --- src/rubicon/objc/api.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/rubicon/objc/api.py b/src/rubicon/objc/api.py index cb8afe0b..ee43a95c 100644 --- a/src/rubicon/objc/api.py +++ b/src/rubicon/objc/api.py @@ -1382,15 +1382,19 @@ class object (in any form accepted by :class:`ObjCInstance`). new_attrs = { "name": objc_class_name, "methods_ptr": None, - # Mapping of name -> method pointer + # Mapping of name -> method pointer. Populated on first attribute access, + # does not contain methods from superclasses. "instance_method_ptrs": {}, - # Mapping of name -> instance method + # Cache of ObjCMethod instance keyed by selector name. Cache misses are + # looked up on instance_method_ptrs for this class and its superclasses. "instance_methods": {}, - # Mapping of name -> (accessor method, mutator method) + # Mapping of name -> (accessor method, mutator method). "instance_properties": {}, # Explicitly declared properties "forced_properties": set(), - # Mapping of first keyword -> ObjCPartialMethod instances + # Cache of ObjCPartialMethod instances keyed by first keyword of the + # selector name. Cache misses are looked up on instance_method_ptrs for this + # class and its superclasses. "partial_methods": {}, # A re-entrant thread lock moderating access to the ObjCClass # method/property cache. This ensures that only one thread populates @@ -1626,10 +1630,10 @@ def _load_methods(self): except KeyError: self.partial_methods[first] = superpartial + # Load methods for this class if not already done. if self.methods_ptr is not None: return - # Load methods for this class. methods_ptr_count = c_uint(0) methods_ptr = libobjc.class_copyMethodList(self, byref(methods_ptr_count)) From 659786b285446c8810d5e0acb107eafa3600ccf7 Mon Sep 17 00:00:00 2001 From: samschott Date: Fri, 6 Dec 2024 12:24:34 +0100 Subject: [PATCH 3/6] add changenote --- changes/547.misc.rst | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 changes/547.misc.rst diff --git a/changes/547.misc.rst b/changes/547.misc.rst new file mode 100644 index 00000000..e69de29b From a4efcc2688b4799afb416f4a28ab87126aa0d635 Mon Sep 17 00:00:00 2001 From: samschott Date: Fri, 6 Dec 2024 12:25:08 +0100 Subject: [PATCH 4/6] add change note content --- changes/547.misc.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/changes/547.misc.rst b/changes/547.misc.rst index e69de29b..bd1deb1c 100644 --- a/changes/547.misc.rst +++ b/changes/547.misc.rst @@ -0,0 +1 @@ +Simplify ObjCClass method loading logic. From 6c8553b9b9e3a4f0ccba2c489cd9379a57a60d5f Mon Sep 17 00:00:00 2001 From: samschott Date: Fri, 6 Dec 2024 12:33:16 +0100 Subject: [PATCH 5/6] correct inline docs for dicts --- src/rubicon/objc/api.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/rubicon/objc/api.py b/src/rubicon/objc/api.py index ee43a95c..8185c1b6 100644 --- a/src/rubicon/objc/api.py +++ b/src/rubicon/objc/api.py @@ -1382,8 +1382,8 @@ class object (in any form accepted by :class:`ObjCInstance`). new_attrs = { "name": objc_class_name, "methods_ptr": None, - # Mapping of name -> method pointer. Populated on first attribute access, - # does not contain methods from superclasses. + # Mapping of name -> method pointer. Populated on first attribute access + # with all methods of this class but **not** its superclasses. "instance_method_ptrs": {}, # Cache of ObjCMethod instance keyed by selector name. Cache misses are # looked up on instance_method_ptrs for this class and its superclasses. @@ -1392,9 +1392,9 @@ class object (in any form accepted by :class:`ObjCInstance`). "instance_properties": {}, # Explicitly declared properties "forced_properties": set(), - # Cache of ObjCPartialMethod instances keyed by first keyword of the - # selector name. Cache misses are looked up on instance_method_ptrs for this - # class and its superclasses. + # ObjCPartialMethod instances keyed by first keyword of the selector name. + # Populated on first attribute access with all methods of this class and its + # superclasses. Updated on misses, e.g., when the class hierarchy changes. "partial_methods": {}, # A re-entrant thread lock moderating access to the ObjCClass # method/property cache. This ensures that only one thread populates From 7e1c648c1c22c91ff31a28d9d966402c245e3af2 Mon Sep 17 00:00:00 2001 From: samschott Date: Sun, 8 Dec 2024 17:51:50 +0100 Subject: [PATCH 6/6] Simplify _cache_property_methods structure --- src/rubicon/objc/api.py | 53 ++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/rubicon/objc/api.py b/src/rubicon/objc/api.py index 8185c1b6..3c68f308 100644 --- a/src/rubicon/objc/api.py +++ b/src/rubicon/objc/api.py @@ -1466,33 +1466,32 @@ def _cache_property_methods(self, name): # If the requested name ends with _, that's a marker that we're # dealing with a method call, not a property, so we can shortcut # the process. - methods = None - else: - # Check 1: Does the class respond to the property? - responds = libobjc.class_getProperty(self, name.encode("utf-8")) - - # Check 2: Does the class have an instance method to retrieve the given name - accessor = self._cache_method(name) - - # Check 3: Is there a setName: method to set the property with the given name - mutator = self._cache_method("set" + name[0].title() + name[1:] + ":") - - # Check 4: Is this a forced property on this class or a superclass? - forced = False - superclass = self - while superclass is not None: - if name in superclass.forced_properties: - forced = True - break - superclass = superclass.superclass - - # If the class responds as a property, or it has both an accessor *and* - # and mutator, then treat it as a property in Python. - if responds or (accessor and mutator) or forced: - methods = (accessor, mutator) - else: - methods = None - return methods + return None + + # Check 1: Does the class respond to the property? + responds = libobjc.class_getProperty(self, name.encode("utf-8")) + + # Check 2: Does the class have an instance method to retrieve the given name + accessor = self._cache_method(name) + + # Check 3: Is there a setName: method to set the property with the given name + mutator = self._cache_method("set" + name[0].title() + name[1:] + ":") + + # Check 4: Is this a forced property on this class or a superclass? + forced = False + superclass = self + while superclass is not None: + if name in superclass.forced_properties: + forced = True + break + superclass = superclass.superclass + + # If the class responds as a property, or it has both an accessor *and* + # and mutator, then treat it as a property in Python. + if responds or (accessor and mutator) or forced: + return accessor, mutator + + return None def _cache_property_accessor(self, name): """Returns a python representation of an accessor for the named