gh-142518: Document thread-safety guarantees of list operations #142519
Conversation
lysnikolaou
force-pushed
the
list-thread-safety-docs
branch
from
1858ef5 to
fd400f0
Compare
|
Merged main to fix CI |
|
I think any note about the atomicity of list operations should probably go after the documentation about the type. I can imagine a beginner to Python going to the documentation to read about lists and being confused about what free-threading semantics are talking about. I also wonder if this should not be a note. Perhaps it would be better to describe as part of the type description what operations are atomic and which are not? |
encukou
left a comment
encukou
left a comment
There was a problem hiding this comment.
This looks like a good start.
Doc/library/stdtypes.rst
Outdated
| guarantees depend on the iterable being passed. When the iterable is | ||
| a :class:`list`, a :class:`tuple`, a :class:`set`, a :class:`frozenset`, | ||
| a :class:`dict` or a :ref:`dictionary view object <dict-views>`, the |
There was a problem hiding this comment.
Does this apply to their subclasses?
There was a problem hiding this comment.
No, only to these classes themselves.
Doc/library/stdtypes.rst
Outdated
|
|
||
| Individual operations on :class:`list` instances such as | ||
| :meth:`~list.append`, :meth:`~list.pop`, ``lst[i] = x``, and ``x = lst[i]`` | ||
| are atomic and will not corrupt the list or crash when called concurrently |
There was a problem hiding this comment.
Consider adding atomic to the glossary, so that it can be linked from all sections like this.
| are atomic and will not corrupt the list or crash when called concurrently | |
| are :term:`atomic` and will not corrupt the list or crash when called concurrently |
Or maybe add a section to the howto instead?
Same for “external synchronization” below.
There was a problem hiding this comment.
"atomic operation" is already part of #140375. Also externally synchronized is part of https://py-free-threading.github.io/documentation-principles/#externally-synchronized. Should we link to that too?
Re. adding a section to the howto, I feel like external synchronization is well within what a user that goes over the free-threading notes will understand. If this gut feeling turns out to be wrong, we can always come back to this and add that section later.
There was a problem hiding this comment.
Please do link the terms. Mental models are built in various ways; paving the pathways is always helpful.
For terms that have a precise definition, it helps to have that readily available. (That's more the case for “atomic” than “external synchronization” I guess.)
|
My general opinion: reference documentation needs to be precise first; complete second. Succinctness comes after that. Edit: Also, IMO, it's fine to add information first, then reorganize when a better structure becomes more obvious. |
lysnikolaou
force-pushed
the
list-thread-safety-docs
branch
from
1a99a5e to
38483c9
Compare
|
I've addressed most of the feedback and added a lot more details. I also inadvertently force-pushed. Sorry about that. |
| The following operations/methods are not fully atomic: | ||
|
|
||
| .. code-block:: | ||
| :class: maybe |
There was a problem hiding this comment.
Maybe this could say something like “Operations/methods that involve iteration are generally not atomic, except when used with specific built-in types”, and iteration itself can be moved here?
Mentioning iteration might help people make sense of this, i.e. it's no longer two arbitrary lists of operations/methods.
Then the bad section below would be left only with examples of “manually” combining multiple operations.
There was a problem hiding this comment.
I don't know how I feel about this. "Operations/methods that involve iteration are generally not atomic" is probably not a mnemonic we want people to use, because there are methods that are atomic but traverse the list. Granted most of those are ones that also mutate it, but e.g. list.copy doesn't.
But the idea of separating iteration from manually combining multiple operations is good. Maybe we should do just that?
4 participants
I'm opening this to start a discussion on what we want the structure of such documentation to be, as well as how to balance detail with succinctness. Feedback very welcome!
📚 Documentation preview 📚: https://cpython-previews--142519.org.readthedocs.build/en/142519/library/stdtypes.html#lists