E534 gh-141510: Avoid critical section on frozendict copy by vstinner · Pull Request #145920 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-141510: Avoid critical section on frozendict copy#145920

Merged
vstinner merged 2 commits intopython:mainfrom
vstinner:optim_frozendict_copy
Mar 16, 2026
Merged

gh-141510: Avoid critical section on frozendict copy#145920
vstinner merged 2 commits intopython:mainfrom
vstinner:optim_frozendict_copy

Conversation

@vstinner
Copy link
Member
@vstinner vstinner commented Mar 13, 2026 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Mar 13, 2026
Open
@vstinner
Copy link
Member Author
vstinner commented Mar 13, 2026

cc @corona10

clin1234
clin1234 reviewed Mar 13, 2026
View reviewed changes
Copy link
Contributor
@clin1234 clin1234 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far, LGTM, though I am curious about concurrency safety in eliding a critical section when copying a frozendict, especially with a free-threaded interpreter

@vstinner
Copy link
Member Author
vstinner commented Mar 16, 2026

Microbenchmark on frozendict | frozendict2 with 50 threads:

Details 
import threading
import time
import pyperf
import sys

KEYS = 4_096
LOOPS = 2 ** 9
NTHREAD = 50


code = f"fd = frozendict({', '.join(f'key{key}=1' for key in range(KEYS))})"
exec(code)
assert(len(fd) == KEYS)
assert(all(sys._is_immortal(key) for key in fd.keys()))
fd2 = frozendict(x=1, y=2, z=3)

class Worker(threading.Thread):
    def __init__(self, fd, fd2):
        super().__init__()
        self.copies = 0
        self.fd = fd
        self.fd2 = fd2

    def run(self):
        fd = self.fd
        fd2 = self.fd2
        for _ in range(LOOPS):
            join = fd | fd2

def bench(fd, fd2):
    threads = [Worker(fd, fd2) for _ in range(NTHREAD)]
    for thread in threads:
        thread.start()
    for thread in threads:
        thread.join()

runner = pyperf.Runner()
runner.bench_func('bench', bench, fd, fd2)

Benchmark result: Mean +- std dev: [ref] 554 ms +- 15 ms -> [optim] 112 ms +- 4 ms: 4.95x faster

I ran the benchmark on Fedora 43 on my laptop with 12 threads (6 CPU cores) on a free-threaded Python build.

@vstinner
Copy link
Member Author
vstinner commented Mar 16, 2026

So far, LGTM, though I am curious about concurrency safety in eliding a critical section when copying a frozendict, especially with a free-threaded interpreter

A frozendict mapping is immutable so a critical section is not needed to ensure that a copy is consistent.

@vstinner vstinner merged commit 77632f0 into python:main Mar 16, 2026
48 checks passed
@vstinner vstinner deleted the optim_frozendict_copy branch March 16, 2026 13:19
@vstinner
Copy link
Member Author
vstinner commented Mar 16, 2026 •
edited
Loading

This change increases the scalability on the following operations:

  • frozendict | frozendict2
  • frozendict.copy() on frozendict subclass
  • _PyDict_CopyAsDict(): used by type() and xml.etree.ElementTree.Element constructors to convert a frozendict to a dict

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

Reviewers

@corona10 corona10 corona10 approved these changes

@methane methane Awaiting requested review from methane methane is a code owner

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

+1 more reviewer

@clin1234 clin1234 clin1234 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vstinner @corona10 @clin1234
0