Skip to content

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

Closed
Closed
json: make "memo" dict local to scan_once call#111928
Assignees
aisk
Labels
3.13only security fixesextension-modulesC modules in the Modules dirtopic-free-threadingtype-featureA feature request or enhancement

Description

@colesbury
colesbury
opened on 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:

cpython/Modules/_json.c

Line 1118 in 289af86

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

Activity

  1. colesbury
    added
    type-featureA feature request or enhancement
    3.13only security fixes
    topic-free-threading
    on Nov 9, 2023
  2. AlexWaygood
    added
    extension-modulesC modules in the Modules dir
    on Nov 10, 2023

  3. aisk commented on Nov 12, 2023

    @aisk
    aisk
    on Nov 12, 2023
    Member

    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.

  4. corona10
    added a commit that references this issue on Nov 13, 2023

    gh-111928: make "memo" dict local to scan_once call (gh-112005)

    d0058cb
  5. colesbury
    assigned
    aisk
    on Nov 13, 2023

  6. colesbury commented on Nov 13, 2023

    @colesbury
    colesbury
    on Nov 13, 2023
    ContributorAuthor

    Thanks @aisk!

  7. colesbury
    closed this as completedon Nov 13, 2023
  8. aisk
    added a commit that references this issue on Feb 11, 2024

    pythongh-111928: make "memo" dict local to scan_once call (pythongh-1…

    1758976
  9. Glyphack
    added a commit that references this issue on Sep 2, 2024

    pythongh-111928: make "memo" dict local to scan_once call (pythongh-1…

    68ce15f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

3.13only security fixesextension-modulesC modules in the Modules dirtopic-free-threadingtype-featureA feature request or enhancement

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions