make self.type a lazy property

Summary: This is the first of two diffs to mitigate creation of unused objects on union creation. This is particularly an issue for containers of unions. The next diff will make `.value` a lazy property, and avoid the python object creation until first time the property is accessed. Reviewed By: Filip-F Differential Revision: D67061643 fbshipit-source-id: 318ab62d78843cda530a24f2ef4fac039cbb60ae
facebook · Dec 11, 2024 · 474af08 · 474af08
1 parent 4bd847c
commit 474af08
Show file tree

Hide file tree

Showing 65 changed files with 444 additions and 261 deletions.
diff --git a/third-party/thrift/src/thrift/compiler/generate/templates/py3/py3_types.pxd.mustache b/third-party/thrift/src/thrift/compiler/generate/templates/py3/py3_types.pxd.mustache
@@ -27,6 +27,7 @@ thrift-py3 cython users.
 {{> types/cimport_thrift }}
 cimport thrift.py3.exceptions
 cimport thrift.py3.types
+from libc.stdint cimport int64_t
 from thrift.python.common cimport (
     RpcOptions as __RpcOptions,
     MetadataBox as __MetadataBox,
@@ -85,7 +86,8 @@ cdef class {{struct:name}}({{> types/python_struct_class}}):
     {{/type:iobufRef?}}{{/field:type}}
     {{/struct:py3_fields}}{{/struct:union?}}
     {{#struct:union?}}
-    cdef readonly object type
+    cdef int64_t type_int
+    cdef object py_type
     cdef readonly object value
     cdef _load_cache({{struct:name}} self)
     {{/struct:union?}}

diff --git a/third-party/thrift/src/thrift/compiler/generate/templates/py3/types.pyx.mustache b/third-party/thrift/src/thrift/compiler/generate/templates/py3/types.pyx.mustache
@@ -363,8 +363,8 @@ cdef class {{struct:name}}(thrift.py3.types.Union):
 
     {{#struct:py3_fields}}
     @property
-    def {{field:py_name}}(self):
-        if self.type.value != {{field:key}}:
+    def {{field:py_name}}({{struct:name}} self not None):
+        if self.type_int != {{field:key}}:
             {{! TODO: python 3.10 adds some more fields to AttributeError, those should be added here at some point }}
             raise AttributeError(f'Union contains a value of type {self.type.name}, not {{field:py_name}}')
         return self.value
@@ -374,13 +374,19 @@ cdef class {{struct:name}}(thrift.py3.types.Union):
     def __hash__({{struct:name}} self):
         return  super().__hash__()
 
+    @property
+    def type({{struct:name}} self not None):
+        if self.py_type is None:
+            self.py_type = {{struct:name}}.Type(self.type_int)
+        return self.py_type
+
     cdef _load_cache({{struct:name}} self):
-        self.type = {{struct:name}}.Type(<int>(deref(self.{{> types/cpp_obj}}).getType()))
-        cdef int type = self.type.value
-        if type == 0:    # Empty
+        self.py_type = None
+        self.type_int = deref(self.{{> types/cpp_obj}}).getType()
+        if self.type_int == 0:    # Empty
             self.value = None
         {{#struct:py3_fields}}
-        elif type == {{field:key}}:
+        elif self.type_int == {{field:key}}:
             {{#field:type}}
             {{> types/cython_union_getter}}
             {{/field:type}}

diff --git a/...rift/src/thrift/compiler/test/fixtures/basic-annotations/out/py3/gen-py3/module/types.pxd b/...rift/src/thrift/compiler/test/fixtures/basic-annotations/out/py3/gen-py3/module/types.pxd
@@ -35,6 +35,7 @@ from thrift.py3.types cimport (
 from thrift.python.common cimport cThriftMetadata as __fbthrift_cThriftMetadata
 cimport thrift.py3.exceptions
 cimport thrift.py3.types
+from libc.stdint cimport int64_t
 from thrift.python.common cimport (
     RpcOptions as __RpcOptions,
     MetadataBox as __MetadataBox,

diff --git a/...mpiler/test/fixtures/basic-enum/out/py3/gen-py3/test/fixtures/enumstrict/module/types.pxd b/...mpiler/test/fixtures/basic-enum/out/py3/gen-py3/test/fixtures/enumstrict/module/types.pxd
@@ -35,6 +35,7 @@ from thrift.py3.types cimport (
 from thrift.python.common cimport cThriftMetadata as __fbthrift_cThriftMetadata
 cimport thrift.py3.exceptions
 cimport thrift.py3.types
+from libc.stdint cimport int64_t
 from thrift.python.common cimport (
     RpcOptions as __RpcOptions,
     MetadataBox as __MetadataBox,

diff --git a/.../src/thrift/compiler/test/fixtures/basic-stack-arguments/out/py3/gen-py3/module/types.pxd b/.../src/thrift/compiler/test/fixtures/basic-stack-arguments/out/py3/gen-py3/module/types.pxd
@@ -35,6 +35,7 @@ from thrift.py3.types cimport (
 from thrift.python.common cimport cThriftMetadata as __fbthrift_cThriftMetadata
 cimport thrift.py3.exceptions
 cimport thrift.py3.types
+from libc.stdint cimport int64_t
 from thrift.python.common cimport (
     RpcOptions as __RpcOptions,
     MetadataBox as __MetadataBox,

diff --git a/.../thrift/compiler/test/fixtures/basic/out/py3/gen-py3/test/fixtures/basic/module/types.pxd b/.../thrift/compiler/test/fixtures/basic/out/py3/gen-py3/test/fixtures/basic/module/types.pxd
@@ -35,6 +35,7 @@ from thrift.py3.types cimport (
 from thrift.python.common cimport cThriftMetadata as __fbthrift_cThriftMetadata
 cimport thrift.py3.exceptions
 cimport thrift.py3.types
+from libc.stdint cimport int64_t
 from thrift.python.common cimport (
     RpcOptions as __RpcOptions,
     MetadataBox as __MetadataBox,
@@ -98,7 +99,8 @@ cdef class MyDataItem(thrift.py3.types.Struct):
 
 cdef class MyUnion(thrift.py3.types.Union):
     cdef shared_ptr[_test_fixtures_basic_module_cbindings.cMyUnion] _cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE
-    cdef readonly object type
+    cdef int64_t type_int
+    cdef object py_type
     cdef readonly object value
     cdef _load_cache(MyUnion self)
 
@@ -158,7 +160,8 @@ cdef class ReservedKeyword(thrift.py3.types.Struct):
 
 cdef class UnionToBeRenamed(thrift.py3.types.Union):
     cdef shared_ptr[_test_fixtures_basic_module_cbindings.cUnionToBeRenamed] _cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE
-    cdef readonly object type
+    cdef int64_t type_int
+    cdef object py_type
     cdef readonly object value
     cdef _load_cache(UnionToBeRenamed self)
 

diff --git a/.../thrift/compiler/test/fixtures/basic/out/py3/gen-py3/test/fixtures/basic/module/types.pyx b/.../thrift/compiler/test/fixtures/basic/out/py3/gen-py3/test/fixtures/basic/module/types.pyx
@@ -574,45 +574,51 @@ cdef class MyUnion(thrift.py3.types.Union):
         return __fbthrift_inst
 
     @property
-    def myEnum(self):
-        if self.type.value != 1:
+    def myEnum(MyUnion self not None):
+        if self.type_int != 1:
             raise AttributeError(f'Union contains a value of type {self.type.name}, not myEnum')
         return self.value
 
     @property
-    def myStruct(self):
-        if self.type.value != 2:
+    def myStruct(MyUnion self not None):
+        if self.type_int != 2:
             raise AttributeError(f'Union contains a value of type {self.type.name}, not myStruct')
         return self.value
 
     @property
-    def myDataItem(self):
-        if self.type.value != 3:
+    def myDataItem(MyUnion self not None):
+        if self.type_int != 3:
             raise AttributeError(f'Union contains a value of type {self.type.name}, not myDataItem')
         return self.value
 
     @property
-    def floatSet(self):
-        if self.type.value != 4:
+    def floatSet(MyUnion self not None):
+        if self.type_int != 4:
             raise AttributeError(f'Union contains a value of type {self.type.name}, not floatSet')
         return self.value
 
 
     def __hash__(MyUnion self):
         return  super().__hash__()
 
+    @property
+    def type(MyUnion self not None):
+        if self.py_type is None:
+            self.py_type = MyUnion.Type(self.type_int)
+        return self.py_type
+
     cdef _load_cache(MyUnion self):
-        self.type = MyUnion.Type(<int>(deref(self._cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE).getType()))
-        cdef int type = self.type.value
-        if type == 0:    # Empty
+        self.py_type = None
+        self.type_int = deref(self._cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE).getType()
+        if self.type_int == 0:    # Empty
             self.value = None
-        elif type == 1:
+        elif self.type_int == 1:
             self.value = translate_cpp_enum_to_python(MyEnum, <int>deref(self._cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE).get_myEnum())
-        elif type == 2:
+        elif self.type_int == 2:
             self.value = MyStruct._create_FBTHRIFT_ONLY_DO_NOT_USE(make_shared[_test_fixtures_basic_module_cbindings.cMyStruct](deref(self._cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE).get_myStruct()))
-        elif type == 3:
+        elif self.type_int == 3:
             self.value = MyDataItem._create_FBTHRIFT_ONLY_DO_NOT_USE(make_shared[_test_fixtures_basic_module_cbindings.cMyDataItem](deref(self._cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE).get_myDataItem()))
-        elif type == 4:
+        elif self.type_int == 4:
             self.value = Set__float__from_cpp(deref(self._cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE).get_floatSet())
 
     def __copy__(MyUnion self):
@@ -1126,21 +1132,27 @@ cdef class UnionToBeRenamed(thrift.py3.types.Union):
         return __fbthrift_inst
 
     @property
-    def reserved_field(self):
-        if self.type.value != 1:
+    def reserved_field(UnionToBeRenamed self not None):
+        if self.type_int != 1:
             raise AttributeError(f'Union contains a value of type {self.type.name}, not reserved_field')
         return self.value
 
 
     def __hash__(UnionToBeRenamed self):
         return  super().__hash__()
 
+    @property
+    def type(UnionToBeRenamed self not None):
+        if self.py_type is None:
+            self.py_type = UnionToBeRenamed.Type(self.type_int)
+        return self.py_type
+
     cdef _load_cache(UnionToBeRenamed self):
-        self.type = UnionToBeRenamed.Type(<int>(deref(self._cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE).getType()))
-        cdef int type = self.type.value
-        if type == 0:    # Empty
+        self.py_type = None
+        self.type_int = deref(self._cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE).getType()
+        if self.type_int == 0:    # Empty
             self.value = None
-        elif type == 1:
+        elif self.type_int == 1:
             self.value = deref(self._cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE).get_reserved_field()
 
     def __copy__(UnionToBeRenamed self):

diff --git a/...y/thrift/src/thrift/compiler/test/fixtures/complex-union/out/py3/gen-py3/module/types.pxd b/...y/thrift/src/thrift/compiler/test/fixtures/complex-union/out/py3/gen-py3/module/types.pxd
@@ -35,6 +35,7 @@ from thrift.py3.types cimport (
 from thrift.python.common cimport cThriftMetadata as __fbthrift_cThriftMetadata
 cimport thrift.py3.exceptions
 cimport thrift.py3.types
+from libc.stdint cimport int64_t
 from thrift.python.common cimport (
     RpcOptions as __RpcOptions,
     MetadataBox as __MetadataBox,
@@ -53,7 +54,8 @@ cdef extern from "thrift/compiler/test/fixtures/complex-union/gen-py3/module/typ
 
 cdef class ComplexUnion(thrift.py3.types.Union):
     cdef shared_ptr[_module_cbindings.cComplexUnion] _cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE
-    cdef readonly object type
+    cdef int64_t type_int
+    cdef object py_type
     cdef readonly object value
     cdef _load_cache(ComplexUnion self)
 
@@ -75,7 +77,8 @@ cdef class ComplexUnion(thrift.py3.types.Union):
 
 cdef class ListUnion(thrift.py3.types.Union):
     cdef shared_ptr[_module_cbindings.cListUnion] _cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE
-    cdef readonly object type
+    cdef int64_t type_int
+    cdef object py_type
     cdef readonly object value
     cdef _load_cache(ListUnion self)
 
@@ -93,7 +96,8 @@ cdef class ListUnion(thrift.py3.types.Union):
 
 cdef class DataUnion(thrift.py3.types.Union):
     cdef shared_ptr[_module_cbindings.cDataUnion] _cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE
-    cdef readonly object type
+    cdef int64_t type_int
+    cdef object py_type
     cdef readonly object value
     cdef _load_cache(DataUnion self)
 
@@ -124,7 +128,8 @@ cdef class Val(thrift.py3.types.Struct):
 
 cdef class ValUnion(thrift.py3.types.Union):
     cdef shared_ptr[_module_cbindings.cValUnion] _cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE
-    cdef readonly object type
+    cdef int64_t type_int
+    cdef object py_type
     cdef readonly object value
     cdef _load_cache(ValUnion self)
 
@@ -142,7 +147,8 @@ cdef class ValUnion(thrift.py3.types.Union):
 
 cdef class VirtualComplexUnion(thrift.py3.types.Union):
     cdef shared_ptr[_module_cbindings.cVirtualComplexUnion] _cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE
-    cdef readonly object type
+    cdef int64_t type_int
+    cdef object py_type
     cdef readonly object value
     cdef _load_cache(VirtualComplexUnion self)
 
@@ -170,7 +176,8 @@ cdef class NonCopyableStruct(thrift.py3.types.Struct):
 
 cdef class NonCopyableUnion(thrift.py3.types.Union):
     cdef shared_ptr[_module_cbindings.cNonCopyableUnion] _cpp_obj_FBTHRIFT_ONLY_DO_NOT_USE
-    cdef readonly object type
+    cdef int64_t type_int
+    cdef object py_type
     cdef readonly object value
     cdef _load_cache(NonCopyableUnion self)