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

json: make "memo" dict local to scan_once call #111928

Closed
Tracked by #108219
colesbury opened this issue Nov 9, 2023 · 2 comments
Closed
Tracked by #108219

json: make "memo" dict local to scan_once call #111928

colesbury opened this issue Nov 9, 2023 · 2 comments
Assignees
Labels
3.13 bugs and security fixes extension-modules C modules in the Modules dir topic-free-threading type-feature A feature request or enhancement

Comments

@colesbury
Copy link
Contributor

colesbury commented Nov 9, 2023

Feature or enhancement

The Modules/_json.c parser is mostly stateless (or the state is immutable). The one exception is the "memo" dictionary, which is used to avoid duplicate PyUnicodeObject instances for the same JSON C strings.

cpython/Modules/_json.c

Lines 696 to 700 in 289af86

memokey = PyDict_SetDefault(s->memo, key, key);
if (memokey == NULL) {
goto bail;
}
Py_SETREF(key, Py_NewRef(memokey));

The memo dictionary is already cleared after each call scan_once:

PyDict_Clear(self->memo);

We should move the creation and destruction of the memo dict to the invocation of scan_once instead of having it as part of the module state. This will avoid contention on the dictionary locks in --disable-gil builds if multiple threads are concurrently parsing JSON strings.

For an example modification, see colesbury/nogil-3.12@964bb33962.

Linked PRs

@aisk
Copy link
Member

aisk commented Nov 12, 2023

I just took a look at the example modification, and run the test on it locally and it succeded. I think there is no more works to do, so just cherry-picked it and made a PR.

If anyone think there need some improves, I want to investigate on it.

corona10 pushed a commit that referenced this issue Nov 13, 2023
Co-authored-by: Sam Gross <colesbury@gmail.com>
@colesbury
Copy link
Contributor Author

Thanks @aisk!

aisk added a commit to aisk/cpython that referenced this issue Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes extension-modules C modules in the Modules dir topic-free-threading type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants