10000 Remove special-casing from `FORMAT_VALUE` opcode · Issue #102598 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

Remove special-casing from FORMAT_VALUE opcode #102598

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.

Sign up for GitHub

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

Jump to bottom
Closed
sobolevn opened this issue Mar 11, 2023 · 1 comment
Closed

Remove special-casing from FORMAT_VALUE opcode #102598

sobolevn opened this issue Mar 11, 2023 · 1 comment
Assignees
@sobolevn
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member
sobolevn commented Mar 11, 2023 •
edited by bedevere-bot
Loading

Right now there's a special case in handling this opcode:

cpython/Python/bytecodes.c

Lines 3060 to 3073 in 5ffdaf7

/* If value is a unicode object, and there's no fmt_spec,
then we know the result of format(value) is value
itself. In that case, skip calling format(). I plan to
move this optimization in to PyObject_Format()
itself. */
if (PyUnicode_CheckExact(value) && fmt_spec == NULL) {
/* Do nothing, just transfer ownership to result. */
result = value;
} else {
/* Actually call format(). */
result = PyObject_Format(value, fmt_spec);
Py_DECREF(value);
Py_XDECREF(fmt_spec);
ERROR_IF(result == NULL, error);

It comes from older version of ceval.c:

cpython/Python/ceval.c

Lines 4379 to 4395 in 2810787

/* If value is a unicode object, and there's no fmt_spec,
then we know the result of format(value) is value
itself. In that case, skip calling format(). I plan to
move this optimization in to PyObject_Format()
itself. */
if (PyUnicode_CheckExact(value) && fmt_spec == NULL) {
/* Do nothing, just transfer ownership to result. */
result = value;
} else {
/* Actually call format(). */
result = PyObject_Format(value, fmt_spec);
Py_DECREF(value);
Py_XDECREF(fmt_spec);
if (result == NULL) {
goto error;
}
}

But, I think that the needed optimizaion is there:

cpython/Objects/abstract.c

Lines 774 to 777 in 5ffdaf7

if (format_spec == NULL || PyUnicode_GET_LENGTH(format_spec) == 0) {
if (PyUnicode_CheckExact(obj)) {
return Py_NewRef(obj);
}

So, I think this can be safely removed.
I will send a PR.

Linked PRs
@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 11, 2023
@sobolevn sobolevn self-assigned this Mar 11, 2023
sobolevn added a commit to sobolevn/cpython that referenced this issue Mar 11, 2023
@sobolevn
pythongh-102598: Remove obsolete optimization from FORMAT_VALUE opcode
52d5125
@bedevere-bot bedevere-bot mentioned this issue Mar 11, 2023
Merged
@hauntsaninja
Copy link
Contributor

Thank you!

hauntsaninja pushed a commit that referenced this issue Mar 21, 2023
@sobolevn
gh-102598: Remove obsolete optimization from FORMAT_VALUE opcode (#…
82eb946 
…102599)
Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this issue Mar 27, 2023
@sobolevn @Fidget-Spinner
pythongh-102598: Remove obsolete optimization from FORMAT_VALUE opc…
af512cc 
…ode (python#102599)
warsaw pushed a commit to warsaw/cpython that referenced this issue Apr 11, 2023
@sobolevn @warsaw
pythongh-102598: Remove obsolete optimization from FORMAT_VALUE opc…
30df57f 
…ode (python#102599)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sobolevn sobolevn

Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@sobolevn @hauntsaninja
0