Use LibC.malloc instead of GC.malloc for external memory allocators

[asterite]

Fixes #11614

There's no need to involve the GC when deciding how memory should be allocated and de-allocated for external libraries. If we use the GC it means that on a GC collect cycle that memory will have to be scanned for pointers... but that memory should never be automatically freed! So it's always better to allocate that memory outside of the GC's knowledge.

[beta-ziliani]

I'm confused with these changes. Why they don't follow the same pattern as the others?

[asterite]

I tried those, they produce a compiler error that I didn't understand 🤷

[asterite]

 > 2 |   LibPCRE.pcre_malloc = ->LibC.malloc(LibC::SizeT)
                               ^
Error: read before assignment to local variable 'arg0'

[asterite]

I forgot to change them to LibC 😊

[HertzDevil]

Wouldn't this now exhaust the memory? I don't think we could really use LibC.free unless we perform manual memory management?

while true
  Regex.new(...)
end

[BlobCodes]

While I agree that the memory allocation should be performed outside of the GC's knowledge, Compress::Deflate is still slower for me using LibC.malloc.
The failing tests are also suspicious and I'm not sure whether all these libraries can be used with LibC malloc.

[asterite]

Wow... I don't understand why this change makes Regex not free its memory. I thought PCRE would call malloc and free when needed, but apparently that's not the case 🤷

[HertzDevil]

PCRE does not perform automatic memory management on behalf of the user. pcre_free exists mainly to ensure that the other deallocation functions from PCRE share the same function (e.g. pcre_free_study).

[lbguilherme]

For Regex we are actually not freeing the regex properly on the finalize method and we were relying on the GC.

class Regex
  def finalize
    LibPCRE.free_study @extra
    LibC.free @re.as(Void*) # <-- this is missing
  end
end

LibPCRE.pcre_malloc = ->(size) { LibC.malloc(size) }
LibPCRE.pcre_free = ->(ptr) { LibC.free(ptr) }
# Or just don't set those functions, by default they will point to LibC.

# This is now fine:
while true
  Regex.new("a|b")
end

All those C libraries are, in theory, free of memory leaks if used properly. The wrappers around them must call the correct free function to ensure everything works.

I think there is value on pursuing this. Not using the GC means less work being done. Probably a irrelevant amount of work though.

[beta-ziliani]

Perhaps it's worth to reconsider it together with Guilherme's patch?

[lbguilherme]

I'm giving it a try.

• LibPCRE: Fairly simple. It needed a patch to ensure the regex is properly freed. I'll open a PR for that.
• LibGMP: Impossible to change. The wrapper types are structs so there is no way to detect when they go out of scope and we have to rely on the GC for the internal data structures.
• LibZ: Small change in that the Reader and Writer need to be closed on finalize. I'll open a PR for that. By the way, I believe every IO should close on finalize... 🤔
• LibXML: Huge patch. The stdlib simply doesn't care about leaking memory and the GC has to collect everything here. I'll check if opening a PR for changing this is feasible.