ibireme / yyjson

The fastest JSON library in C
https://ibireme.github.io/yyjson/doc/doxygen/html/
MIT License
3.04k stars 262 forks source link

Change realloc function signature #100

Closed lnkuiper closed 1 year ago

lnkuiper commented 1 year ago

Is your feature request related to a problem? Please describe. Currently, the function signature of yyjson_alc's realloc is the following:

void *(*realloc)(void *ctx, void *ptr, size_t size);

I want to plug a custom allocator, but this allocator needs to know the size of the originally allocated memory region.

Describe the solution you'd like I would like for realloc's function signature to be changed to:

void *(*realloc)(void *ctx, void *ptr, size_t old_size, size_t size);

And all usages of this function should be correctly passed the old size.

Describe alternatives you've considered I have considered adding the following to my custom allocator to be able to get the size of the allocated memory region:

std::unordered_map<void *, size_t> ptr_size_map;

However, changing the function signature would be more elegant.

Additional context Thanks again for this amazing library. We use it in our JSON extension for DuckDB. The feature I am requesting would greatly reduce the number of allocations when combined with our ArenaAllocator. We parse one vector (=2048) of JSON documents at a time rather than the more common use case of one document at a time, therefore yyjson_alc_pool_init's usefulness is limited.

ibireme commented 1 year ago

Thanks for the feedback. It isn't difficult to modify, but this is a breaking change. Let me think again if there is another better way to achieve your needs.

lnkuiper commented 1 year ago

Thank you for the quick response. Would this change could be non-breaking if it is wrapped it like this?

void *realloc(void *ctx, void *ptr, size_t old_size, size_t size) {
   return realloc_without_old_size ? realloc_without_old_size(ctx, ptr, size) : realloc_with_old_size(ctx, ptr, old_size, size);
}

Internally, all calls to realloc can be replaced to supply the old_size parameter, but existing custom allocator implementations don't have to implement the new function signature.

ibireme commented 1 year ago

I tried it and felt it made the allocator API more complicated, so let's make a breaking change. Documentation and Changelog have also been updated: https://github.com/ibireme/yyjson/commit/31db654f24ac30d02f7959a73d0a79bebe992440

lnkuiper commented 1 year ago

You're amazing! Thanks for making this change. I look forward to testing it out this week

lnkuiper commented 1 year ago

I have refactored our JSON extension following the realloc changes, and it works like a charm! Thanks again.

DuckDB Labs would like to thank you for making such an awesome library. Please e-mail me at laurens@duckdblabs.com and we can send you some merch if you're interested :)

ibireme commented 1 year ago

I have refactored our JSON extension following the realloc changes, and it works like a charm! Thanks again.

DuckDB Labs would like to thank you for making such an awesome library. Please e-mail me at laurens@duckdblabs.com and we can send you some merch if you're interested :)

Thank you for using my library and for your kind words. I'm glad to hear that it was helpful with your product~