Skip to content

bpo-34152: Improved performance of list for a[::-1] = a, a[i:j] = a and a[len(a):] = b.#8333

Closed
sir-sigurd wants to merge 4 commits intopython:masterfrom
sir-sigurd:slice-same-list
Closed

bpo-34152: Improved performance of list for a[::-1] = a, a[i:j] = a and a[len(a):] = b.#8333
sir-sigurd wants to merge 4 commits intopython:masterfrom
sir-sigurd:slice-same-list

Conversation

@sir-sigurd
Copy link
Copy Markdown
Contributor

@sir-sigurd sir-sigurd commented Jul 18, 2018

Copy link
Copy Markdown
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments here but I am not sure how i feel about this, as the code is much more complex only for these special cases. Specially the case for a[i:j] = a can be a bit confusing if you don't see before hand the operation strategy.

Comment thread Objects/listobject.c Outdated
size_t s;
int result = -1; /* guilty until proved innocent */

if (ilow < 0)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, use braces as stated by PEP7.

Comment thread Objects/listobject.c Outdated

if (ilow < 0)
ilow = 0;
else if (ilow >= Py_SIZE(a))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use braces here (see previous message).

Comment thread Objects/listobject.c Outdated
else if (ilow >= Py_SIZE(a))
ilow = Py_SIZE(a);

if (ihigh < ilow)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use braces here (see previous message).

Comment thread Objects/listobject.c Outdated

if (ihigh < ilow)
ihigh = ilow;
else if (ihigh > Py_SIZE(a))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use braces here (see previous message).

Comment thread Objects/listobject.c Outdated
for (k = 0; k < ilow; k++) {
PyObject *obj = a->ob_item[k];
Py_INCREF(obj);
a->ob_item[k + ilow] = obj;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: I prefer a->ob_item[ilow + k] = obj; as I always tend to read this as point_of_origin + moving_index . Also is consistent with the rest of the movements.

@bedevere-bot
Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@pablogsal
Copy link
Copy Markdown
Member

Closed: see https://bugs.python.org/issue34152 for rationale.

Thanks for the proposal!

@pablogsal pablogsal closed this May 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants