Skip to content

Conversation

@donbarbos
Copy link
Contributor

@donbarbosdonbarbos commented May 20, 2025

@donbarbos
Copy link
ContributorAuthor

cc @picnixz

@picnixzpicnixz added the needs backport to 3.14 bugs and security fixes label May 20, 2025
Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

I don't know if the parameters are meant to be public but the signature indeed needs to be correct for possible subclassing.

However, prepare_context is not documented so we can't even know how to use them. So we need to also document this at the same time.

@donbarbos
Copy link
ContributorAuthor

I don't know if the parameters are meant to be public but the signature indeed needs to be correct for possible subclassing.

ctxkwargs argument described in ThreadPoolExecutor.__init__ docstring.

And maybe I'm missing something but this is what the class method prepare_context looks like (I don't know how **ctxkwargs should be passed here if this argument is omitted in prepare_context):

@classmethod
defprepare_context(cls, initializer, initargs):
returnWorkerContext.prepare(initializer, initargs)
def__init__(self, max_workers=None, thread_name_prefix='',
initializer=None, initargs=(), **ctxkwargs):
"""Initializes a new ThreadPoolExecutor instance.
Args:
max_workers: The maximum number of threads that can be used to
execute the given calls.
thread_name_prefix: An optional name prefix to give our threads.
initializer: A callable used to initialize worker threads.
initargs: A tuple of arguments to pass to the initializer.
ctxkwargs: Additional arguments to cls.prepare_context().
"""
ifmax_workersisNone:
# ThreadPoolExecutor is often used to:
# * CPU bound task which releases GIL
# * I/O bound task (which releases GIL, of course)
#
# We use process_cpu_count + 4 for both types of tasks.
# But we limit it to 32 to avoid consuming surprisingly large resource
# on many core machine.
max_workers=min(32, (os.process_cpu_count() or1) +4)
ifmax_workers<=0:
raiseValueError("max_workers must be greater than 0")
(self._create_worker_context,
self._resolve_work_item_task,
) =type(self).prepare_context(initializer, initargs, **ctxkwargs)

@picnixz
Copy link
Member

And maybe I'm missing something but this is what the class method prepare_context looks like (I don't know how **ctxkwargs should be passed here if this argument is omitted in prepare_context):

in this case, we should wait for Eric's feedback. I'm away until Saturday so I won't be able to reply in the meantime.

@donbarbos
Copy link
ContributorAuthor

Thanks everyone for suggestions, now it looks complete

pending jobs will raise a :exc:`~concurrent.futures.thread.BrokenThreadPool`,
as well as any attempt to submit more jobs to the pool.

*ctxkwargs* is a mapping of additional keyword arguments passed to

Choose a reason for hiding this comment

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

The ctxkwargs parameter is special -- it does not correspond to a single argument. I am not even sure that it should be documented, since it is only purpose is to be able to pass shared to prepare_context() in InterpreterPoolExecutor(). It may be an implementation detail, in which case I am not sure that it should be documented.

If we still document this, I would write something like "Any addition keyword arguments are passed to prepare_context()." And updated the docstring correspondingly.

cc @ericsnowcurrently

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

Labels

awaiting reviewdocsDocumentation in the Doc dirneeds backport to 3.14bugs and security fixesskip news

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

4 participants

@donbarbos@picnixz@serhiy-storchaka@StanFromIreland