enthought / enthought/comtypes

Can we use `IsEqualGUID` in the `GUID.__eq__`?

Open
#659 4 comments 0 reactions 0 assignees View on GitHub
good first issue
Dominant language
Python
Stars
345
Forks
105
PR merge metrics
No merged PRs in 30d

Description

The module defining `GUID` includes the following "Note":

https://github.com/enthought/comtypes/blob/083c19e9fdc0f366df48ad448e897246f346ede0/comtypes/_post_coinit/guid.py#L26-L27

However, this note was written at the very start of this project, even before the release of Python 3.0.

CPython and `ctypes` have improved over the past years.
Is there a performance difference between the current implementation and an implementation with [`IsEqualGUID`](https://learn.microsoft.com/en-us/windows/win32/api/guiddef/nf-guiddef-isequalguid) like the following?

```diff
@@ -13,6 +13,7 @@ def binary(obj: "GUID") -> bytes:
BYTE = c_byte
WORD = c_ushort
DWORD = c_ulong
+TRUE = 1

_ole32 = oledll.ole32

@@ -22,9 +23,7 @@ _ProgIDFromCLSID = _ole32.ProgIDFromCLSID
_CLSIDFromString = _ole32.CLSIDFromString
_CLSIDFromProgID = _ole32.CLSIDFromProgID
_CoCreateGuid = _ole32.CoCreateGuid
-
-# Note: Comparing GUID instances by comparing their buffers
-# is slightly faster than using ole32.IsEqualGUID.
+_IsEqualGUID = _ole32.IsEqualGUID

class GUID(Structure):
@@ -50,7 +49,7 @@ class GUID(Structure):
return self != GUID_null

def __eq__(self, other) -> bool:
- return isinstance(other, GUID) and binary(self) == binary(other)
+ return isinstance(other, GUID) and _IsEqualGUID(byref(self), byref(other)) == TRUE

def __hash__(self) -> int:
# We make GUID instances hashable, although they are mutable.
```

After comparing benchmarks, if it proves effective, feel free to submit a PR.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.