ENH: Followup audit of FT build

[HaoZeke]

cc @ngoldbaum

The main changes here are basically due to not trying to mix C allocator and Python memory allocator via memset or memcpy.

• Switched Pool.alloc to use PyMem_Calloc, removing memset
• Updated Pool.realloc to use PyMem_Realloc

This PR is a draft until:

[ ] CI for testing and building wheels with Python 3.13t is fully green.
[ ] More static audits of the code

[rgommers]

If you're adding to the public API, you should explain in the PR description why. Also, calloc method is added to the implementation but not here.

[ngoldbaum]

Can you turn actions on for your fork to make sure CI passes?

[ngoldbaum]

Did you try running e.g. the example use-case from the readme on the GIL-enabled and free-threaded build?

[rgommers]

Can you turn actions on for your fork to make sure CI passes?

I did this when reviewing gh-49, and it wasn't working for several reasons. I opened gh-51 to fix things up. With that, gh-50 passes. I didn't check this PR.

[HaoZeke]

Did you try running e.g. the example use-case from the readme on the GIL-enabled and free-threaded build?

I had built the example and tested it a bit before these changes but since it doesn't actually have any methods defined on the sparse matrix I basically checked that creation of a bunch of sparse matrices in a list comprehension works. No failure then. With this PR I'll double check but I think it was "working" the same way (i.e. no confidence in it either way). I'll try to flesh out the sparse matrix example with a few operations.

[ngoldbaum]

Looking at the reverse dependencies, I only recognize thinc. You could look at how that library is using cymem.

The original comment was just towards making sure this is getting tested for a real-ish use-case, given the lack of tests for most functionality,

[HaoZeke]

Looking at the reverse dependencies, I only recognize thinc. You could look at how that library is using cymem.

The original comment was just towards making sure this is getting tested for a real-ish use-case, given the lack of tests for most functionality,

Sadly thinc has a dep, but seems to be an unused one.

https://github.com/explosion/thinc/blob/20eec35566997d5272d083be1f1a0ec9006934ff/thinc/backends/numpy_ops.pyx#L10

Will go through the rest :)

EDIT: Bit of a trend here, most of the libraries don't seem to use it at all, e.g. mdparse, etk

[honnibal]

Unfortunately this will be more disruptive than a reasonable person would expect.

We build against cymem across our dependency tree (we cimport cymem), and in order to add a member, you need to add a cdef in the .pxd.

If a public class changes size in Cython, it's no longer binary-compatible. This means we'll need to make this a major version increment, and we'd need to do updates across our dependency chain to use the new version.

[honnibal]

This is backwards incompatible. A user who was instantiated the class with something like mem = Pool(pymalloc=Default_Malloc, pyfree=Default_Free) would suffer segfaults.

When adding new arguments to a function with defaults, the new arguments need to be placed after the current ones.

[rgommers]

The main changes here are basically due to not trying to mix C allocator and Python memory allocator via memset or memcpy.

I think the backwards-compatible way to do this, assuming it's actually necessary to do this and to support a single Pool instance being used from multiple threads, would be to add a new class with the changed allocation and API, so packages using this can switch over at their leisure if and when they need this for free-threading support.

[rgommers]

gh-53 is now merged, which addresses free-threading support fully and in a backwards compatible way. So I'll go ahead and close this. Thanks for your efforts @HaoZeke.