Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve custom allocator support #9

Open
golightlyb opened this issue Apr 4, 2015 · 11 comments
Open

Improve custom allocator support #9

golightlyb opened this issue Apr 4, 2015 · 11 comments

Comments

@golightlyb
Copy link

Hi, thanks for LodePNG.

To make the custom allocator functionality (LODEPNG_NO_COMPILE_ALLOCATORS) useful to me, I need the lodepng_malloc/realloc/free functions to accept an extra void *user_arg argument.

I think this can be done without breaking the API by e.g.

unsigned lodepng_decode_memory_using(unsigned char** out, unsigned* w, unsigned* h,
                               const unsigned char* in, size_t insize,
                               LodePNGColorType colortype, unsigned bitdepth, void *allocator_arg)
{
    // normal implementation except using allocator_arg for lodepng_malloc, lodepng_free etc.
}

unsigned lodepng_decode_memory(unsigned char** out, unsigned* w, unsigned* h,
                               const unsigned char* in, size_t insize,
                               LodePNGColorType colortype, unsigned bitdepth)
{
    // wrapper that behaves like the default
    return lodepng_decode_memory_using(out, w, h, in, size, colortype, bitdepth, NULL);
}

#ifdef LODEPNG_COMPILE_ALLOCATORS
static void* lodepng_malloc(size_t size, void *user_arg)
{
    UNUSED(user_arg);
    return malloc(size);
}

However this would break the API for anyone already using custom allocators.

Would such a change, in principle, be accepted by you? If so I'm prepared to make the changes for you and send you a pull request. Many thanks!

@lvandeve
Copy link
Owner

lvandeve commented Apr 5, 2015

Hi,

I'll do it in an update soon. I've indeed seen similar allocators with void argument used elsewhere.

Out of curiosity, what do you need it for?

Thanks

@lvandeve
Copy link
Owner

It does require a lot of changes to pass the user object through all code paths.

If lodepng is called only once at the time, maybe the object can be made global instead so your custom function can get it from there? Would that work for now?

@ghost
Copy link

ghost commented Nov 26, 2015

@golightlyb what is that void *user_arg good for? For what do you use it? I'm just curious.

@AntTheAlchemist
Copy link

Be careful not to add too much bulk to LodePNG. It's beauty is that it's interface is simple and compact. If we understand what the user_arg is for, it will help decide?

I'm using LODEPNG_NO_COMPILE_ALLOCATORS to link into my garbage collection logic that overrides new & delete in C++. But the problem for me is new & delete can't realloc, so I've had to add padding to manage realloc requests. Is there any chance to disable realloc requests?

@lvandeve
Copy link
Owner

It'd probably be through the state object, not an extra function argument, if I do it. Not sure when though.

But actually, indeed maybe it's not needed at all? The required state for allocations can be stored in the memory you manage. Is that right?

@AntTheAlchemist
Copy link

Hi Lode (nice to meet you).

I'm not sure I understand your question about state objects.

I override new and delete to manage memory. When new fails to allocate, I call a special method that deletes memory (in my case, cached resources such as textures) until the allocation is successful. This guarantees that new will never fail, so I don't need to check every time I make a new object.

So, I use LODEPNG_NO_COMPILE_ALLOCATORS to use my custom new & delete operators so that lodepng never fails to allocate. Works very well. But to make lodepng_realloc() work, I have to store a size_t for every lodepng_malloc(), so I know how much memory to memcpy() into the new_size. It works, but it's a little messy.

This is how I'm storing the size_t:

void * lodepng_malloc(size_t size) { char *n = new char[size + sizeof(size_t)]; *(size_t*)n = size; n += sizeof(size_t); return n; }

No big problem, but without the need for realloc, this would be a lot cleaner for anyone trying to do what I'm doing in C++.

@lvandeve
Copy link
Owner

realloc is quite convenient :)

What is the reason for not being able to use it?

AFAIK realloc is often able to run in constant time because if memory behind it was free anyway it can immediately use it. Do your allocators take this into account?

Do you use any C++ features like std::strings or std::vectors that also need to allocate more memory sometimes? How do you use those?

Thanks!

@AntTheAlchemist
Copy link

What if malloc() or realloc() fail because there is no free memory? I have no way to catch the error and free memory for it to try again.

I could, instead of using 'new', use malloc() and realloc() and call my garbage collection if they fail, then retry, but these allocations are outside of my control if I am not using new & delete operators, which have no way to realloc, which is why in lodepng_realloc() I use new char[new_size], memcpy(), delete old ptr;

I'm not using std::strings or std::vectors. Will the memory they dynamically allocate get caught in my overridden 'new' operator, so I can implement garbage collection on them if they fail due to out of memory?

@lvandeve
Copy link
Owner

LodePNG will return an error number 83 in case any allocation fails, if that helps :)

LodePNG only allocates primitive types, so C's malloc and free should do.

@AntTheAlchemist
Copy link

Actually, that's a good idea. If LodePNG returns 83, I can release some memory and retry. That's even simpler than before, lol.

Okay, forget about dropping realloc. All is ok :)

@AntTheAlchemist
Copy link

Just a quick suggestion. Any plans to make constants or #defines for the error codes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants