simplejson icon indicating copy to clipboard operation
simplejson copied to clipboard

Initial free threading port

Open mitsuhiko opened this issue 2 months ago • 11 comments

Triggered by this tweet I was really curious if Claude can implement free threading support on my bike ride to the office. The answer seems to be: yes.

This now is a much more complex implementation than the initial commit which only removed the GIL. This implementation now is makes it both sub-interpreter and free-threading ready. Given that the code also still supports Python 2.x there are now quite a few paths to take care of.

Important notes: the standard library's JSON module uses a macro called Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST and PySequence_Fast. The former uses private APIs which are not exposed and as such we have no option to do that. The new code special cases lists and tuples and then falls back to the iterator interface. I'm generally not sure if that is the right approach but this is what some back and forth with codex and claude suggested as the optimal solutions given the constraints.

I'm actually not entirely convinced because I did not run benchmarks.

Unfortunately this change overall drives up the complexity significantly given the many versions that this code needs to support. For instance in Python 2 I don't think there is a way to avoid the static types so the code also needs to support that.

The code as it is produced right now is ~50% Claude, ~40% Codex and ~10% manual edits and has been going through multiple iterations of Codex based reviews. I have manually reviewed it a few times now but if you think that approach makes any sense, I would probably make a proper manual pass over still.

mitsuhiko avatar Oct 20 '25 08:10 mitsuhiko

Are all of the statics valid in free threading? I haven’t read through the required changes but I would imagine it’d be better to have per-interpreter references rather than process globals

etrepum avatar Oct 20 '25 14:10 etrepum

Free threading just gets rid of the GIL. The subinterpreter changes move the GIL on a per-subinterpreter level which means that objects can not be shared. However you are right that these changes do not allow simplejson to be loaded into subinterpreters. That would be a separate change.

mitsuhiko avatar Oct 20 '25 15:10 mitsuhiko

I will have a look at the review comments. I had claude refactor the module to carry through a module state but that turned out to be much more involved than I expected. Going to have a look at the review comments and if I can cut down on some of that noise.

mitsuhiko avatar Oct 20 '25 18:10 mitsuhiko

Disregard the last commit, I need to do more changes.

mitsuhiko avatar Oct 20 '25 18:10 mitsuhiko

This has become rather hard to review because I had to move this all into heap types. Most of this is still based on what Claude put out, but I had to do some manual edits along the way to move it towards success.

I don't have a Python 2.7, so I hope that this passes in CI.

mitsuhiko avatar Oct 20 '25 19:10 mitsuhiko

This will require a different change because it turns out that Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST is not public :(

mitsuhiko avatar Oct 20 '25 23:10 mitsuhiko

Probably worth reading through some of Python's current json module to see what it does.

The unfortunate reality is that the stdlib uses Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST which is private. I'm honestly not sure what the correct approach is here.

mitsuhiko avatar Oct 21 '25 08:10 mitsuhiko

I think for iteration all that's needed is to essentially copy that macro? AFAICT _PyCriticalSection_Begin is just the static inline internal version of PyCriticalSection_BeginMutex.

https://github.com/python/cpython/blob/main/Include/internal/pycore_critical_section.h#L29-L42

etrepum avatar Oct 21 '25 15:10 etrepum

@etrepum unfortunately those APIs are private and not part of the stable ABI. That will only change with 3.15: https://github.com/python/cpython/pull/135899

mitsuhiko avatar Oct 22 '25 07:10 mitsuhiko

It looks like it was backported to 3.14 according to that issue?

etrepum avatar Oct 22 '25 13:10 etrepum

https://github.com/python/cpython/blob/v3.14.0/Include/cpython/critical_section.h#L76-L77

etrepum avatar Oct 22 '25 16:10 etrepum