Skip to content

Commit 591b62b

Browse files
ahilgerfacebook-github-bot
authored andcommitted
simplify Struct.__call__
Summary: We can remove some duplicate code and make `__call__` faster by feeding `kwargs` to `_initStructTupleWithValues`. The deleted section in the `else` of `__call__` duplicates the logic in `_initStructTupleWithValues`. In the new flow, we have 3 cases: - `kwarg` specified as None -> `_initStructTupleWithValues` sets to default value - `kwarg is non-`None` -> `_initStructTupleWithValues` sets to new value - `kwarg` not specified -> `__call__` borrows a new reference to old value Reviewed By: prakashgayasen, createdbysk Differential Revision: D74365485 fbshipit-source-id: b25226de15cb7da1b68cb660fa058d8065bee3f0
1 parent aa3c3f7 commit 591b62b

File tree

2 files changed

+8
-28
lines changed

2 files changed

+8
-28
lines changed

third-party/thrift/src/thrift/lib/python/types.pyx

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,42 +1106,22 @@ cdef class Struct(StructOrUnion):
11061106
return self
11071107
cdef StructInfo struct_info = self._fbthrift_struct_info
11081108
klass = type(self)
1109-
# NB: optimization opportunity here: pass **kwargs to fbthrift_new
1110-
cdef Struct new_inst = klass._fbthrift_new()
1109+
# note this puts the set kwargs into new_inst._fbthrift_data
1110+
cdef Struct new_inst = klass._fbthrift_new(**kwargs)
11111111
not_found = object()
11121112
isset_flags = self._fbthrift_data[0]
11131113

11141114
for field_name, field_index in struct_info.name_to_index.items():
11151115
value = kwargs.pop(field_name, not_found)
11161116
if value is None: # reset to default value, no change needed
11171117
continue
1118-
if value is not_found: # copy the old value if needed
1118+
if value is not_found: # borrow ref to old value
11191119
if isset_flags[field_index] == 0:
11201120
# old field not set, so keep default
11211121
continue
1122-
value_to_copy = self._fbthrift_data[field_index + 1]
1123-
else: # new assigned value
1124-
try:
1125-
field_spec = struct_info.fields[field_index]
1126-
adapter_info = field_spec.adapter_info
1127-
if adapter_info:
1128-
adapter_class, transitive_annotation = adapter_info
1129-
field_id = field_spec.id
1130-
value = adapter_class.to_thrift_field(
1131-
value,
1132-
field_id,
1133-
self,
1134-
transitive_annotation=transitive_annotation(),
1135-
)
1136-
value_to_copy = (
1137-
<TypeInfoBase>struct_info.type_infos[field_index]
1138-
).to_internal_data(value)
1139-
except Exception as exc:
1140-
raise type(exc)(
1141-
f"{type(self)}: error updating Thrift struct field "
1142-
f"'{field_spec.py_name}': {exc}"
1143-
) from exc
1144-
set_struct_field(new_inst._fbthrift_data, field_index, value_to_copy)
1122+
borrowed_value = self._fbthrift_data[field_index + 1]
1123+
set_struct_field(new_inst._fbthrift_data, field_index, borrowed_value)
1124+
11451125
if kwargs:
11461126
raise TypeError(
11471127
f"'{type(self).__name__}' object does not have attribute(s): "

third-party/thrift/src/thrift/test/thrift-python/struct_test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ def test_call(self) -> None:
165165
with self.assertRaisesRegex(
166166
TypeError,
167167
(
168-
"error updating Thrift struct field 'unqualified_string': Cannot create "
168+
"error .* Thrift struct field 'unqualified_string': Cannot create "
169169
"internal string data representation. Expected type <class 'str'>, got: "
170170
"<class 'int'>."
171171
),
@@ -429,7 +429,7 @@ def test_adapted_types(self) -> None:
429429
with self.assertRaisesRegex(
430430
AttributeError,
431431
(
432-
"error updating Thrift struct field "
432+
"error .* Thrift struct field "
433433
"'unqualified_adapted_i32_to_datetime': 'int' object has no attribute "
434434
"'timestamp'"
435435
),

0 commit comments

Comments
 (0)