Repository navigation
gh-140328: Use interned versions of string constants if they're already present #140688
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
base: main
Are you sure you want to change the base?
Changes from all commits
113db5f
4cc6757
7bf7281
57a9f65
ab137ba
f09b691
5cbe0bf
291cc27
919539f
87759b4
9ecb016
10c48c2
7ec88fc
b130563
6438774
f3f65cc
4ba8a5c
1d0071a
a433644
1f0b319
70a825e
254fe8a
a767680
52e9e9a
7088626
5ddf0a5
5e907c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import re | ||
| from pathlib import Path | ||
|
|
||
| from typing import Iterable | ||
|
|
||
| from test.support.project_files_helper import iter_all_c_files | ||
|
|
||
|
|
||
| # copypaste from 'Tools/build/generate_global_objects.py' | ||
| def iter_global_strings() -> Iterable[str]: | ||
| id_regex = re.compile(r"\b_Py_ID\((\w+)\)") | ||
| str_regex = re.compile(r'\b_Py_DECLARE_STR\((?:\w+), "(.*?)"\)') | ||
| for filename in iter_all_c_files(): | ||
| infile = Path(filename) | ||
| if not infile.exists(): | ||
| # The file must have been a temporary file. | ||
| continue | ||
| with infile.open(encoding="utf-8") as infile_open: | ||
| for line in infile_open: | ||
| for m in id_regex.finditer(line): | ||
| yield m.group(1) | ||
| for m in str_regex.finditer(line): | ||
| yield m.group(1) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| from pathlib import Path | ||
|
|
||
| from typing import Iterable | ||
|
|
||
| ROOT = Path(__file__).resolve().parents[3] | ||
|
|
||
|
|
||
| # copypaste from 'Tools/build/generate_global_objects.py' | ||
| def iter_all_c_files() -> Iterable[Path]: | ||
| for top_directory_name in ( | ||
| "Modules", | ||
| "Objects", | ||
| "Parser", | ||
| "PC", | ||
| "Programs", | ||
| "Python", | ||
| ): | ||
| for dirname, _, files in (ROOT / top_directory_name).walk(): | ||
| for name in files: | ||
| if not name.endswith((".c", ".h")): | ||
| continue | ||
| yield dirname / name |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Added lookups to global and per-interpreter tables of interned strings while creating codeobjects, so some of the string constants could be reused |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -202,18 +202,37 @@ intern_strings(PyObject *tuple) | |
| static int | ||
| intern_constants(PyObject *tuple, int *modified) | ||
| { | ||
| #define set_modified(modified) if (modified) *modified = 1 | ||
|
|
||
| PyInterpreterState *interp = _PyInterpreterState_GET(); | ||
| for (Py_ssize_t i = PyTuple_GET_SIZE(tuple); --i >= 0; ) { | ||
| PyObject *v = PyTuple_GET_ITEM(tuple, i); | ||
| if (PyUnicode_CheckExact(v)) { | ||
| if (PyUnicode_CHECK_INTERNED(v) != 0) { | ||
| continue; | ||
| } | ||
| #if !defined(Py_GIL_DISABLED) | ||
| // borrowed reference | ||
| PyObject *interned = _Py_hashtable_get(INTERNED_STRINGS, v); | ||
| if (interned == NULL) { | ||
| interned = PyDict_GetItemWithError(get_interned_dict(interp), v); | ||
| if (PyErr_Occurred()) { | ||
| return -1; | ||
| } | ||
| } | ||
| if (interned != NULL && interned != v) { | ||
| Py_INCREF(interned); | ||
| PyTuple_SET_ITEM(tuple, i, interned); | ||
| Py_DECREF(v); | ||
| set_modified(modified); | ||
| } else | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it adds extra indentation (that I'd like to avoid) to the next code block like so: } else {
#endif
if (should_intern_string(v)) {
PyObject *w = v;
_PyUnicode_InternMortal(interp, &v);
if (w != v) {
PyTuple_SET_ITEM(tuple, i, v);
set_modified(modified);
}
}
}
}
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, let's wait what say others for this case. |
||
| #endif | ||
| if (_PyCode_ShouldInternString(v)) { | ||
| PyObject *w = v; | ||
| _PyUnicode_InternMortal(interp, &v); | ||
| if (w != v) { | ||
| PyTuple_SET_ITEM(tuple, i, v); | ||
| if (modified) { | ||
| *modified = 1; | ||
| } | ||
| set_modified(modified); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -242,9 +261,7 @@ intern_constants(PyObject *tuple, int *modified) | |
|
|
||
| PyTuple_SET_ITEM(tuple, i, v); | ||
| Py_DECREF(w); | ||
| if (modified) { | ||
| *modified = 1; | ||
| } | ||
| set_modified(modified); | ||
| } | ||
| Py_DECREF(tmp); | ||
| } | ||
|
|
@@ -273,9 +290,7 @@ intern_constants(PyObject *tuple, int *modified) | |
| } | ||
| PyTuple_SET_ITEM(tuple, i, v); | ||
| Py_DECREF(slice); | ||
| if (modified) { | ||
| *modified = 1; | ||
| } | ||
| set_modified(modified); | ||
| } | ||
| Py_DECREF(tmp); | ||
| } | ||
|
|
@@ -293,14 +308,14 @@ intern_constants(PyObject *tuple, int *modified) | |
| else if (interned != v) { | ||
| PyTuple_SET_ITEM(tuple, i, interned); | ||
| Py_SETREF(v, interned); | ||
| if (modified) { | ||
| *modified = 1; | ||
| } | ||
| set_modified(modified); | ||
| } | ||
| } | ||
| #endif | ||
| } | ||
| return 0; | ||
|
|
||
| #undef set_modified | ||
| } | ||
|
|
||
| /* Return a shallow copy of a tuple that is | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.