FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-125420: implement `Sequence.count` API on `memoryview` objects by picnixz · Pull Request #125443 · python/cpython · GitHub

Repository navigation

gh-125420: implement Sequence.count API on memoryview objects - #125443

Merged
JelleZijlstra merged 15 commits into
python:mainfrom
picnixz:fix/memoryview-sequence-api-count-125420
Dec 10, 2024
Merged

JelleZijlstra merged 15 commits into
python:mainfrom
picnixz:fix/memoryview-sequence-api-count-125420

Conversation

picnixz commented Oct 14, 2024 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

picnixz changed the title gh-125420: add count to memoryview objects gh-125420: implement Sequence.count API on memoryview objects Oct 14, 2024
Comment thread Objects/memoryobject.c
Comment thread Objects/memoryobject.c
Py_DECREF(iter);
return NULL;
}
int contained = PyObject_RichCompareBool(item, value, Py_EQ);

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

list.count has a special case for if (item == value) count++. I suspect that helps significantly here too, because the value item will usually be an interned integer.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Actually, PyObject_RichCompareBool has that fast path inside as well. I had the fast path before but then I wondered whether it was really needed or not (and I was a bit too lazy for benchmarkings). But I forgot about the interned integer case so it's probably faster.

Comment thread Objects/memoryobject.c

Py_ssize_t count = 0;
PyObject *item = NULL;
while (PyIter_NextItem(iter, &item)) {

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We could probably make this significantly more efficient by looking at the memory directly without materializing an iterator and all the elements. str.count is a similar example. However, that is going to be a lot more complicated, so it seems fine to skip it; if somebody wants to optimize this method later they can.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, that's what I suspected so I went for the simplest approach (at least to have the code around).

JelleZijlstra enabled auto-merge (squash) December 10, 2024 02:52
JelleZijlstra merged commit 4331832 into python:main Dec 10, 2024
picnixz deleted the fix/memoryview-sequence-api-count-125420 branch December 10, 2024 10:13
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL