Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test comprehensions and generator expressions with loop optimisations #4967

Open
scoder opened this issue Aug 9, 2022 · 1 comment
Open

Comments

@scoder
Copy link
Contributor

scoder commented Aug 9, 2022

Generator expressions and comprehensions seem to need a lot more testing, especially in combination with all the loop optimisations that we have: lists/tuples, range(), enumerate(), bytes/bytearray/unicode, memoryviews, C arrays, you name it.

At least the bytearray iteration fails to optimise inside of a generator expression, probably others.

This seems worth adding as dedicated test modules, e.g. genexpr_loop_optimisations_py.py (and possibly genexpr_loop_optimisations_cy.pyx).

Found as part of the change in #4254 (which broke memoryview iteration).

@da-woods
Copy link
Contributor

da-woods commented Aug 9, 2022

To me the bytearray iteration looks OK in a generator expression

Just as a short example

def f(x: bytearray):
    count = 0
    for y in x:
        count += 1
    return (y==count for y in x)

Snippet of the C code for the generator:

__pyx_t_2 = PyByteArray_GET_SIZE(__pyx_t_1); if (unlikely(__pyx_t_2 == ((Py_ssize_t)-1))) __PYX_ERR(0, 7, __pyx_L1_error)
  for (__pyx_t_4 = 0; __pyx_t_4 < __pyx_t_2; __pyx_t_4++) {
    __pyx_t_3 = __pyx_t_4;
    __pyx_cur_scope->__pyx_v_y = ((unsigned char)(PyByteArray_AS_STRING(__pyx_t_1)[__pyx_t_3]));

and for the plain loop:

__pyx_t_2 = PyByteArray_GET_SIZE(__pyx_t_1); if (unlikely(__pyx_t_2 == ((Py_ssize_t)-1))) __PYX_ERR(0, 5, __pyx_L1_error)
  for (__pyx_t_4 = 0; __pyx_t_4 < __pyx_t_2; __pyx_t_4++) {
    __pyx_t_3 = __pyx_t_4;
    __pyx_v_y = ((unsigned char)(PyByteArray_AS_STRING(__pyx_t_1)[__pyx_t_3]));

So they're doing substantially the same thing - getting the size with PyByteArray_GET_SIZE and indexing into the C string from PyByteArray_AS_STRING.

Doesn't mean it shouldn't be tested of course (or that you haven't found a slightly different case where it doesn't optimize)

@scoder scoder removed this from the 3.0 milestone Jul 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants