Skip to content

Commit

Permalink
Speed up enum comparisons
Browse files Browse the repository at this point in the history
Summary:
- special case situation when types of `self` and `other` values are the same
- save on attribute access and instead treat object as Python int using  `PyLong_AsLong` to read numeric value. NOTE: we only do this enums that have int as base class (bypassing flag enums that don't have the same underlying representation)

#buildall

Benchmark results (subsequent diff)

```
# baseline
int (baseline): start 100000000 comparisons
Finished in: 6.380038888193667
enum: start 100000000 comparisons
Finished in: 19.200770954135805

# check types + use attribute access
int (baseline): start 100000000 comparisons
Finished in: 6.133592534810305
enum: start 100000000 comparisons
Finished in: 11.616515398956835

# check types + PyLong_AsLong
int (baseline): start 100000000 comparisons
Finished in: 6.21840969985351
enum: start 100000000 comparisons
Finished in: 8.569516903720796

```

Reviewed By: Filip-F

Differential Revision: D66850779

fbshipit-source-id: a2adaa542a5097f2707f34db43ecea1d6cec3f81
  • Loading branch information
Vladimir Matveev authored and facebook-github-bot committed Dec 12, 2024
1 parent 8a45458 commit 0993c97
Showing 1 changed file with 28 additions and 15 deletions.
43 changes: 28 additions & 15 deletions thrift/lib/python/types.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ from cpython.object cimport Py_LT, Py_EQ, PyCallable_Check
from cpython.tuple cimport PyTuple_New, PyTuple_SET_ITEM, PyTuple_GET_ITEM, PyTuple_Check
from cpython.set cimport PyFrozenSet_New, PySet_Add
from cpython.ref cimport Py_INCREF, Py_DECREF
from cpython.long cimport PyLong_AsLong
from cpython.unicode cimport PyUnicode_AsUTF8String, PyUnicode_FromEncodedObject
from cython.operator cimport dereference as deref

Expand Down Expand Up @@ -2292,12 +2293,16 @@ class EnumMeta(type):
for name, value in dct.items():
if not isinstance(value, int):
attrs[name] = value

klass = super().__new__(
metacls,
classname,
bases,
attrs,
)
if int in bases:
type.__setattr__(klass, "__eq__", Enum.__int__eq__)

for name, value in dct.items():
if not isinstance(value, int):
continue
Expand Down Expand Up @@ -2361,6 +2366,23 @@ cdef inline _fbthrift_enum_equivalent(a, b):
cdef int b_types = b_module.rfind(".")
return a_module[:a_types] == b_module[:b_types]

cdef inline bint _enum_eq_(self, other):
if isinstance(other, Enum):
if self is other:
return True
# handle py3 vs python comparison
return (
self._fbthrift_value_ == other._fbthrift_value_ and
_fbthrift_enum_equivalent(self, other)
)
if cFollyIsDebug and isinstance(other, (bool, float)):
warnings.warn(
f"Did you really mean to compare {type(self)} and {type(other)}?",
RuntimeWarning,
stacklevel=1
)
return self._fbthrift_value_ == other

class Enum(metaclass=EnumMeta):
def __init__(self, _):
# pass on purpose to keep the __init__ interface consistent with the other base class (i.e. int)
Expand Down Expand Up @@ -2391,21 +2413,7 @@ class Enum(metaclass=EnumMeta):
return type(self), (self.value,)

def __eq__(self, other):
if isinstance(other, Enum):
if self is other:
return True
# handle py3 vs python comparison
return (
self._fbthrift_value_ == other._fbthrift_value_ and
_fbthrift_enum_equivalent(self, other)
)
if cFollyIsDebug and isinstance(other, (bool, float)):
warnings.warn(
f"Did you really mean to compare {type(self)} and {type(other)}?",
RuntimeWarning,
stacklevel=1
)
return self._fbthrift_value_ == other
return _enum_eq_(self, other)

# thrift-python enums have int base, so have to define
# __ne__ to avoid __ne__ based on int value alone
Expand All @@ -2430,6 +2438,11 @@ class Enum(metaclass=EnumMeta):
def __bool__(self):
return True

def __int__eq__(self, other):
if type(self) is type(other):
return PyLong_AsLong(self) == PyLong_AsLong(other)
return _enum_eq_(self, other)


class Flag(Enum):
@classmethod
Expand Down

0 comments on commit 0993c97

Please sign in to comment.