Application-supplied memory allocators

[YakoYakoYokuYoku]

Because of the usage of the Zend memory manager in the PHP interpreter, gd has a situation with having many options for memory management and no control over it.

This PR brings an API for the management that corresponds to the following methods:

void gdSetMemoryCallocMethod(gdCallocf callocf);
void gdSetMemoryMallocMethod(gdMallocf mallocf);
void gdSetMemoryReallocMethod(gdReallocf reallocf);
void gdSetMemoryFreeMethod(gdFreef freef);
void gdSetMemoryAllocationMethods(gdCallocf callocf, gdMallocf mallocf, gdReallocf reallocf, gdFreef freef);

Each one of these sets an specific allocator for the task and there is a much more general method that sets the allocation functions at once.
The underlying magic is setting static variables for every method.
All of gdMalloc and its friends had to be modified to use the variables.

Marked as draft because discussions may take place first. Fixes #335.

[cmb69]

Thank you! Looks generally good to me, but maybe it's better to split the CMake/pthread changes into a separate PR?

[YakoYakoYokuYoku]

You welcome.

Pinging @vapier for review.

[YakoYakoYokuYoku]

Maybe reallocf is not a good name.

[cmb69]

It may make sense to use the same terms: gdSetMemoryCallocMethod vs. gdCallocf. Why not gdCallocMethod? Or gdSetMemoryCallocFunc and gdCallocFunc.

[vapier]

lot of boilerplate, but not sure we can do better :/. splattering a lot of defines feels like it might be fewer LOC, but harder to understand.

[YakoYakoYokuYoku]

Running the test with CMake works just fine but not with Autotools 🤔 .

[pierrejoye]

Very good PR , thank you! :)

It could be a tat bit better using:

common struct to store the f

• function ptr data in a global struct, surrounded by our lock mechanism, Each getter/setter using the lock/unlock
• must be only called once per process (not sure we can ensure it but then doc issue, big mess if it changes accross the same process)

What do you think?

[YakoYakoYokuYoku]

From what was discussed in the issue we can go either ways with the static struct or the vars, but I'll stick with the latter for the moment.
Speaking about locking I find it kinda pointless, the methods are obviously not thread safe and will cause disastrous alterations with the memory managers if handled incorrectly.
Maybe doing checks on another global variable is an option. Or maybe we could not check anything and document that calling twice is invalid usage (kinda like the vkBeginCommandBuffer docs).

[pierrejoye]

I think it would make sense to have a struct, it will also helps CPU cache, especially in loaders where quite a few alloc/free may happen. Or in GD3 with the Paths APIs.

Taste thing, I find it cleaner as well :)

What do you think?

[pierrejoye]

Besides that, I am all for it and sounds like a good addition for 2.4.0 :) Well done!!

[YakoYakoYokuYoku]

Only thing about this PR is that testing gdmem with Autotools and shared libgd leads to the functions not being used in the test. Although it works with an static archive. From there idek what's happening.

[cmb69]

I think it would make sense to have a struct, it will also helps CPU cache, especially in loaders where quite a few alloc/free may happen. Or in GD3 with the Paths APIs.

I don't have a strong opinion on this, but sounds reasonable.

Most important for me is that we ship custom allocator support ASAP. I wouldn't want to unbundle libgd from php-src before this isn't available.

Only thing about this PR is that testing gdmem with Autotools and shared libgd leads to the functions not being used in the test. Although it works with an static archive.

Thanks for reporting. I think this needs closer investigation.

Anyhow, if you have some time, could you please resolve the merge conflicts?

[YakoYakoYokuYoku]

No further comments @pierrejoye, @cmb69, @vapier?

[pierrejoye]

@YakoYakoYokuYoku in principle I am good with it. I was waiting @remicollet feedback on the API.

While I still like to provide the atomicity handling on our side tho'. That can be done post merge.

[remicollet]

I was waiting @remicollet feedback on the API.

I don't really care about the API ;) (as soon as it is there)

BTW, a single setter may be enough (I don't think changing only some make sense, and a single call with 4 pointers will avoid forgetting one).

Perhaps a safety bell checking those allocators have not be used yet before allowing the switch...

[YakoYakoYokuYoku]

@cmb69, @pierre, @remicollet, @vapier I'm deeply sorry for being late with this PR, was occupied with things IRL and I didn't know how to fix this PR up until now.

So if anyone of you can merge this I'd be glad for it. 🙏