Repository navigation
Conversation
JukkaL
left a comment
There was a problem hiding this comment.
Looks good overall, I just had some style nits.
| check = f"{check} && !({bitmap} & {bit})" | ||
| self.emit_line(f"if ({check}) {{") | ||
|
|
||
| def emit_tuple_attr_load( |
There was a problem hiding this comment.
It would be good for the name to make it clear that this a synchronized load.
| self.emit_line("Py_END_CRITICAL_SECTION();") | ||
| return undefined | ||
|
|
||
| def emit_tuple_attr_store( |
There was a problem hiding this comment.
Similar to above, update name?
| delete: bool = False, | ||
| is_init: bool = False, | ||
| ) -> None: | ||
| """Publish a tuple under the owner lock, releasing old references outside it.""" |
There was a problem hiding this comment.
The above description is a bit odd: "Publish", "releasing old references outside it". Can you make it clearer?
| """, | ||
| ) | ||
|
|
||
| def test_final_tuple_attr_reads(self) -> None: |
There was a problem hiding this comment.
This test case seems to roughly replicate the emitted logic, which seems kind of pointless. If there is some important case that should be covered, I think it's better to have an unconditional emit. Not every combination needs to be covered here.
There was a problem hiding this comment.
ok i've deleted this one and added a simpler one to check that a final attribute doesn't take the lock on free-threaded builds.
| is_init: bool = False, | ||
| ) -> None: | ||
| """Publish a tuple under the owner lock, releasing old references outside it.""" | ||
| """Assign a tuple attribute attr to src under the owner lock. |
There was a problem hiding this comment.
This seems reversed -- should be assign src to a tuple attribute?
Work on mypyc/mypyc#1203
Take a critical section when accessing fixed-length tuple attributes on free-threaded builds. Fields of these tuples are stored in a C struct so concurrent updates could result in readers observing a state in the middle of the update. Also use atomic operations on bitmaps on free-threaded builds.
Final tuple attributes are unaffected. I saw pretty much no difference in mypy self-check benchmark on 3.14t.