Skip to content

PERF: Remove use of generators in Cython code #898

Closed as not planned
Closed as not planned
PERF: Remove use of generators in Cython code#898
Assignees
cpcloud
Labels
P1Medium priority - Should docuda.bindingsEverything related to the cuda.bindings moduleenhancementAny code-related improvements

Description

@mdboom
mdboom
opened on Aug 25, 2025

Is this a duplicate?

Area

General cuda-python

Is your feature request related to a problem? Please describe.

Generators create very inefficient code in Cython, relative to a simple C for loop, and the overhead is especially bad when the sequences are small.

Describe the solution you'd like

We should remove instances of generators where ever possible.

Describe alternatives you've considered

No response

Additional context

No response

Activity

  1. mdboom
    self-assigned this
    on Aug 25, 2025
  2. github-actions
    added
    triageNeeds the team's attention
    on Aug 25, 2025

  3. kkraus14 commented on Aug 25, 2025

    @kkraus14
    kkraus14
    on Aug 25, 2025
    Collaborator

    Generators create very inefficient code in Cython, relative to a simple C for loop, and the overhead is especially bad when the sequences are small.

    Do you have any sense as to how hard it would be to improve this upstream? We should be able to adopt new Cython versions relatively easily.

  4. mdboom commented on Aug 25, 2025

    @mdboom
    mdboom
    on Aug 25, 2025
    ContributorAuthor

    I'm not sure. In our specific case, Cython is creating the most general code possible here that could iterate over anything, but we "know" from the type annotations that it's always a list or a tuple (which should be much easier to iterate over). I don't know if the Cython compiler has enough context already to special case for that and make the transformation. I'll poke around and see...

  5. mdboom commented on Aug 25, 2025

    @mdboom
    mdboom
    on Aug 25, 2025
    ContributorAuthor

    Some specific examples that create generators now:

    if not all(isinstance(_x, (CUlibraryOption)) for _x in libraryOptions):
    raise TypeError("Argument 'libraryOptions' is not instance of type")

    is better as:

    for _x in libraryOptions:
    if not isinstance(_x, (CUlibraryOption)):
        raise TypeError("Argument 'libraryOptions' is not instance of type")

    During generated class construction every class has something like:

    _dict_CUipcMem_flags = dict(((int(v), v) for k, v in CUipcMem_flags.__members__.items()))

    Since the class is code-generated by cython-gen anyway, this would be much better as:

    _dict_CUipcMem_flags = { 
    int(CU_IPC_MEM_LAZY_ENABLE_PEER_ACCESS): CU_IPC_MEM_LAZY_ENABLE_PEER_ACCESS 
}

    because it can take advantage of knowing the fixed size of the class etc. (But these changes would only affect import times, not run times).

    Additionally, if it matters, these could probably benefit from a mapping where the keys are always integers, rather than a general Python dictionary.

  6. leofang commented on Aug 25, 2025

    @leofang
    leofang
    on Aug 25, 2025
    Member

    Additionally, if it matters, these could probably benefit from a mapping where the keys are always integers, rather than a general Python dictionary.

    Does Cython already support optimizing dict access with known kv-types? I am not aware of this capability, if so it'd be great! Or maybe you have in mind c++ dicts?

    Another place where we could enjoy optimization is to rely on the fact that these error-code dicts are read-only after construction, but again I am not aware of Cython being able to optimize for this use case.

  7. mdboom commented on Aug 26, 2025

    @mdboom
    mdboom
    on Aug 26, 2025
    ContributorAuthor

    Does Cython already support optimizing dict access with known kv-types? I am not aware of this capability, if so it'd be great! Or maybe you have in mind c++ dicts?

    No, I don't think it does -- I was suggesting we would pull from somewhere else (probably the C++ stdlib).

    Another place where we could enjoy optimization is to rely on the fact that these error-code dicts are read-only after construction, but again I am not aware of Cython being able to optimize for this use case.

    Yeah, that's a good point. I /think/ the keys are even compile-time constants, so you could use something like a perfect hash library (though that's probably overkill).

  8. cpcloud commented on Aug 26, 2025

    @cpcloud
    cpcloud
    on Aug 26, 2025
    Contributor

    I'm happy to take this on as a good first issue. @mdboom Are you actively working on this at the moment?

  9. mdboom commented on Aug 27, 2025

    @mdboom
    mdboom
    on Aug 27, 2025
    ContributorAuthor

    I'm happy to take this on as a good first issue. @mdboom Are you actively working on this at the moment?

    No, go for it!

  10. cpcloud commented on Aug 29, 2025

    @cpcloud
    cpcloud
    on Aug 29, 2025
    Contributor

    It seems that generator expressions are not performant, but comparing the codegen for a list comprehension versus an append-in-for-loop approach I couldn't see any meaningful difference in the generated code.

  11. leofang
    assigned
    cpcloud
    and unassigned
    mdboom
    on Aug 30, 2025
  12. leofang
    added
    enhancementAny code-related improvements
    P1Medium priority - Should do
    cuda.bindingsEverything related to the cuda.bindings module
    triageNeeds the team's attention
    and removed
    triageNeeds the team's attention
    on Aug 30, 2025

  13. mdboom commented on Sep 2, 2025

    @mdboom
    mdboom
    on Sep 2, 2025
    ContributorAuthor

    It seems that generator expressions are not performant, but comparing the codegen for a list comprehension versus an append-in-for-loop approach I couldn't see any meaningful difference in the generated code.

    Cool. Thanks for the confirmation.

  14. leofang
    linked a pull request that will close this issueperf: remove generator expressions from input validation loops #940on Sep 3, 2025

  15. leofang commented on Sep 3, 2025

    @leofang
    leofang
    on Sep 3, 2025
    Member

    @mdboom @cpcloud Question: We are changing

    if not all(isinstance(x, something) for x in a_long_sequence):
    ...

    to

    for x in a_long_sequence:
    if not isinstance(x, something):
        ...

    which is basically changing all to any (which has an early return). Is this the main reason we improve perf, or is Cython just really bad at compiling generator code?

  16. mdboom commented on Sep 3, 2025

    @mdboom
    mdboom
    on Sep 3, 2025
    ContributorAuthor

    which is basically changing all to any (which has an early return). Is this the main reason we improve perf, or is Cython just really bad at compiling generator code?

    We could test the theory by transforming to any(not isinstance..., but I don't think that's the source of the difference.

    The main difference is that Python generators are extremely general and operate by making numerous C functions calls, so would be hard for Cython to optimize. Maybe if it has enough context about the type it could transform it into a for loop, but I don't think it currently does that.

  17. leofang commented on Sep 16, 2025

    @leofang
    leofang
    on Sep 16, 2025
    Member

    Closing since we could not prove in #940 that using a plain for loop actually helps improve the performance, at least not with the benchmark scripts that we crafted there. We'll have to revisit this when there's a more realistic benchmark.

  18. leofang
    closed this as not plannedon Sep 16, 2025
  19. leofang
    removed
    triageNeeds the team's attention
    on Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

P1Medium priority - Should docuda.bindingsEverything related to the cuda.bindings moduleenhancementAny code-related improvements

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions