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

str.is* implementation seem suboptimal for single character strings #61759

Closed
gdementen mannequin opened this issue Mar 27, 2013 · 6 comments
Closed

str.is* implementation seem suboptimal for single character strings #61759

gdementen mannequin opened this issue Mar 27, 2013 · 6 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@gdementen
Copy link
Mannequin

gdementen mannequin commented Mar 27, 2013

BPO 17559
Nosy @birkenfeld, @vstinner, @benjaminp, @ezio-melotti, @serhiy-storchaka

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2013-03-28.09:56:32.591>
created_at = <Date 2013-03-27.14:41:09.281>
labels = ['interpreter-core', 'performance']
title = 'str.is* implementation seem suboptimal for single character strings'
updated_at = <Date 2013-03-28.09:56:32.589>
user = 'https://bugs.python.org/gdementen'

bugs.python.org fields:

activity = <Date 2013-03-28.09:56:32.589>
actor = 'georg.brandl'
assignee = 'none'
closed = True
closed_date = <Date 2013-03-28.09:56:32.591>
closer = 'georg.brandl'
components = ['Interpreter Core']
creation = <Date 2013-03-27.14:41:09.281>
creator = 'gdementen'
dependencies = []
files = []
hgrepos = []
issue_num = 17559
keywords = []
message_count = 6.0
messages = ['185338', '185392', '185393', '185394', '185407', '185410']
nosy_count = 6.0
nosy_names = ['georg.brandl', 'vstinner', 'benjamin.peterson', 'ezio.melotti', 'gdementen', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'works for me'
stage = None
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue17559'
versions = ['Python 3.4']

@gdementen
Copy link
Mannequin Author

gdementen mannequin commented Mar 27, 2013

In isspace, isalpha, isalnum and isdigit, I see code like:

/* Shortcut for single character strings */
if (PyString_GET_SIZE(self) == 1 &&
isspace(*p))
return PyBool_FromLong(1);

Is it intentional to not use:

if (PyString_GET_SIZE(self) == 1))
return PyBool_FromLong(isspace(*p) != 0);

which would be faster when the result is False (but a tad slower when it is True because of the extra comparison).

Also, is there a reason (other than historical) why the macros Py_RETURN_TRUE and Py_RETURN_FALSE are not used instead of their equivalent functions PyBool_FromLong(1) and PyBool_FromLong(0)?

See:
http://hg.python.org/cpython/file/e87364449954/Objects/stringobject.c#l3324

@gdementen gdementen mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Mar 27, 2013
@benjaminp
Copy link
Contributor

The shortcut seems fairly pointless to me.

@vstinner
Copy link
Member

If you would like to improve Python, you have to focus on the development version which is Python 3.4. In this version, the code is different:

    if (length == 1)
        return PyBool_FromLong(
            Py_UNICODE_ISSPACE(PyUnicode_READ(kind, data, 0)));

I'm not sure that having a special case for string of 1 character provide any speed up...

@benjaminp
Copy link
Contributor

There's still stuff in bytes_methods.c which looks like the old string code.

@gdementen
Copy link
Mannequin Author

gdementen mannequin commented Mar 28, 2013

Argl. I know I should have used 3.4... but that is what I thought I did.
I used http://hg.python.org/cpython then "browse", and assumed it was the default branch... I know realize that since the last commit at that time was on the 2.7 branch, that is what I got. And I didn't even realize my mistake by the string vs unicode differences. Now, I will just go hide somewhere in shame...

@birkenfeld
Copy link
Member

Benjamin, if that comment means there's still something to be done, please reopen.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
Projects
None yet
Development

No branches or pull requests

3 participants