Skip to content

bpo-24700: Add a fast path for comparing array.array of equal type#3009

Merged
pitrou merged 1 commit into
python:masterfrom
adrian17:fast-path-v4
Aug 17, 2017
Merged

bpo-24700: Add a fast path for comparing array.array of equal type#3009
pitrou merged 1 commit into
python:masterfrom
adrian17:fast-path-v4

Conversation

@adrian17

@adrian17 adrian17 commented Aug 6, 2017 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

pitrou
pitrou reviewed Aug 8, 2017
View reviewed changes
Comment thread Modules/arraymodule.c Outdated

@pitrou pitrou Aug 8, 2017

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.

It confused me that this can be jumped into even when sizes are equal. I would suggest renaming equal_except_size to something else. Perhaps compare_sizes?

@pitrou pitrou Aug 12, 2017

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.

Looks like this change was reversed.

@pitrou

pitrou commented Aug 8, 2017

Copy link
Copy Markdown
Member

This lacks a NEWS entry as produced by the blurb utility. Could you add one?

@pitrou

pitrou commented Aug 12, 2017

Copy link
Copy Markdown
Member

One thing: how does this fare on arrays of NaNs? Currently we have:

>>> a = array.array("d", [float('nan')])
>>> b = array.array("d", [float('nan')])
>>> a == b
False
>>> a > b
False
>>> a < b
False

@adrian17

adrian17 commented Aug 12, 2017

Copy link
Copy Markdown
Contributor Author

Right, didn't check to handle it. I added a test case for NaN - even if the optimization doesn't get merged, the test case would still be nice to have.

I realized I also missed the PyFPE_START_PROTECT macros. All in all, properly handling floats would either complicate the code a lot, or make the fast path slower in general, so I decided I'd rather just disable the fast path for floats.

@pitrou

pitrou commented Aug 12, 2017

Copy link
Copy Markdown
Member

All in all, properly handling floats would either complicate the code a lot, or make the fast path slower in general, so I decided I'd rather just disable the fast path for floats.

Fair enough. Someone else can re-add a fast path for floats if they want to.

@pitrou pitrou merged commit 7c17e23 into python:master Aug 17, 2017
@adrian17 adrian17 deleted the fast-path-v4 branch August 17, 2017 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitrou pitrou pitrou left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@adrian17 @pitrou @the-knights-who-say-ni