Skip to content

RFC: Change __cuda_stream__ from class attribute to method #348

Closed

Description

@leofang

One of the reasons that people complain about CUDA Array Interface (CAI) being slow is because it has side effects. Consider this example:

>>> class KKK:
...     @property
...     def __proppp__(self):
...             print("I am called")
... 
>>> 
>>> type(KKK.__proppp__)
<class 'property'>
>>> k = KKK()
>>> hasattr(k, "__proppp__")
I am called
True

One can see that the hasattr call on any class attribute/property would actually execute the attribute implementation, but not if it is a method:

>>> class KKK:
...     def __proppp__(self):
...             print("I am called")
... 
>>> 
>>> type(KKK.__proppp__)
<class 'function'>
>>> k = KKK()
>>> hasattr(k, "__proppp__")
True

I realized I had forgotten about this during the design stage until now. This fact has also been pointed out by @gmarkall (back in the days of designing CAI, IIRC).

I propose to change __cuda_stream__ and any future protocols to a callable class method from a class property/attribute so as to avoid side effects (thereby reducing overhead) and establish a future-proof convention, following DLPack and other protocols.

Activity

  1. added
    breakingBreaking changes are introduced
    cuda.coreEverything related to the cuda.core module
    RFCPlans and announcements
    triageNeeds the team's attention
    on Jan 3, 2025
  2. self-assigned this
    on Jan 3, 2025
  3. bdice commented on Jan 4, 2025

    @bdice
    Contributor

    To add some context: this is the intended design of hasattr. https://docs.python.org/3/library/functions.html#hasattr

    The arguments are an object and a string. The result is True if the string is the name of one of the object’s attributes, False if not. (This is implemented by calling getattr(object, name) and seeing whether it raises an AttributeError or not.)

    @property decorators create descriptors, which are one of many ways to add dynamic attributes to an object. Unfortunately, the only way to resolve such dynamic attribute lookups is by actually trying to get the attribute, so we are left paying this cost when using properties.

    Relevant StackOverflow post: https://stackoverflow.com/questions/30143957/why-does-hasattr-execute-the-property-decorator-code-block

    I ran some microbenchmarks and I agree that using a method instead of a property is a good solution to increase performance, regardless of whether the user takes an EAFP (try: ... except AttributeError:) or LBYL (if hasattr(...):) approach.

  4. added
    P0High priority - Must do!
    and removed
    triageNeeds the team's attention
    on Jan 9, 2025
  5. added this to the cuda.core beta 3 milestone on Jan 9, 2025
  6. leofang commented on Jan 11, 2025

    @leofang
    MemberAuthor

    @NaderAlAwar, this should be a quick fix, could you help make the change and propagate it to cuda.par PR please? 🙂

  7. shwina commented on Jan 11, 2025

    @shwina
    Contributor

    @bdice Just out of curiosity, I added a couple of more tests to your benchmark (cached properties, getattr_static):

    Click to see benchmark
    import time
    import timeit
    
    setups = {
        "cached_property": """
    from functools import cached_property
    class MyClass:
        @cached_property
        def __protocol__(self):
            time.sleep(0.00001)
            return True
    obj = MyClass()""",
        "property": """
    class MyClass:
        @property
        def __protocol__(self):
            time.sleep(0.00001)
            return True
    
    obj = MyClass()""",
        "method": """
    class MyClass:
        def __protocol__(self):
            time.sleep(0.00001)
            return True
    
    obj = MyClass()""",
        "none": """
    class MyClass:
        pass
    
    obj = MyClass()""",
    }
    
    tests = {
        "eafp": """
    try:
        obj.__protocol__
    except AttributeError:
        pass
        """,
        "lbyl": """hasattr(obj, "__protocol__")""",
        "gentle-lbyl": (
            """"__protocol__" in obj.__dir__() or hasattr(obj, "__protocol__")"""
        ),
        "getattr_static": """
    import inspect
    inspect.getattr_static(obj, "__protocol__", None)
    """,
    }
    
    results = {}
    for setup_name, setup_stmt in setups.items():
        for test_name, test_stmt in tests.items():
            test_name = f"{setup_name}, {test_name}"
            times = timeit.repeat(
                setup=setup_stmt,
                stmt=test_stmt,
                repeat=10,
                number=1000,
                globals={"time": time},
            )
            avg_time = sum(times) / len(times)
            results[test_name] = avg_time
    
    for test_name, result in sorted(results.items(), key=lambda x: x[1]):
        print(f"{test_name:>18}: {result:03g}")

    Method access is still fastest (although cached properties is a very close second).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

P0High priority - Must do!RFCPlans and announcementsbreakingBreaking changes are introducedcuda.coreEverything related to the cuda.core module

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions