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

Custom Memory Allocator Methods #584

Closed
GavinLaz opened this issue Feb 24, 2022 · 7 comments
Closed

Custom Memory Allocator Methods #584

GavinLaz opened this issue Feb 24, 2022 · 7 comments

Comments

@GavinLaz
Copy link

Libjpeg-turbo uses malloc/free internally for its memory allocation. While large allocations can be avoided by providing pre-sized buffers, each call to tjInitCompress or tjInitDecompress does a small allocation (and then free when tjDestroy is called). When used in the same process over a long period of time, this constant churn of malloc/free can produce memory fragmentation.

My request is to create an API that allows a developer to set custom alloc/free methods (with the same signatures as malloc/free). This would allow a developer to use a memory pool or do other custom handling.

Potential interface:

struct CustomMemoryMethods
{
  void* (*cbAlloc)(size_t size);
  void (*cbFree)(void* memory);
};

void tjSetCustomMemoryMethods(CustomMemoryMethods*);
@dcommander
Copy link
Member

Questions:

  1. Can you provide more information about the circumstances under which memory fragmentation could occur with the existing library? (A realistic, reproducible test case would be even better.) I am aware of how memory fragmentation can occur in theory, but in practice, it seems unlikely to occur with the TurboJPEG API library unless you are calling tjInit*Compress() a bunch of times without calling tjDestroy(). I can't think of any good reason why an application would do that.
  2. Why can't you just reuse the handle returned from tjInitCompress() or tjInitDecompress()? You don't technically need to create a new handle unless you need to compress/decompress multiple images simultaneously in multiple threads.

Unfortunately, it isn't as simple as introducing a new TurboJPEG function. The TurboJPEG API is a wrapper for the libjpeg API, and the libjpeg API allocates memory in a bunch of different places. libjpeg originally shipped with different memory manager implementations for various platforms, but with the exception of the malloc()/free() implementation that libjpeg-turbo currently uses, all of the others were for obsolete platforms (MS-DOS, etc.) The architecture of the libjpeg API library is such that the memory manager implementation can't be replaced at run time. With a bit of re-architecture, it should be possible to introduce new libjpeg API functions that allow jpeg_get_small(), jpeg_get_large(), jpeg_free_small(), and jpeg_free_large() to be overridden with application-supplied callback functions, the addresses of which could be stored in the opaque master structure. Then the memory manager could call those callback functions, if they are specified, rather than their library-supplied counterparts. (Refer to #308 and #325.) That would be a non-trivial effort, though, and it would be necessary to complete it before similar functionality could be added to the TurboJPEG API.

Any major new TurboJPEG API feature is best implemented as part of the TurboVNC v3 API overhaul (#517), which is an even more non-trivial effort. The other hell of it is that a global state-setting function such as you propose above wouldn't be appropriate, because the thread safety of that function could only be ensured on platforms that support thread-local storage. (That includes most of them these days, but not all, and I really don't want to make a major part of the TurboJPEG API dependent on TLS. One of the goals in the API overhaul is to eliminate global functions and make everything in the API stateful.) Thus, it would be possible to override the memory allocation/de-allocation functions on a per-instance basis, but tjInit*Compress()/tjDestroy() would still have to use malloc() and free(). So if that's the crux of your issue, then all of that non-trivial effort still wouldn't solve it.

@GavinLaz
Copy link
Author

GavinLaz commented Feb 24, 2022 via email

@dcommander
Copy link
Member

Since your use case isn't common and you were able to work around it in a different way, I will close the feature request for now. For anyone stumbling upon this issue, I will note that:

  • There is some potential utility in overriding the memory manager's alloc/free functions, but that would have to be done at the libjpeg API level first. (IOW, this enhancement would have a dependency of Question: overriding jpeg_get_large #308 or Provide memory manager overriding api #325.)
  • To avoid duplication of effort, this enhancement would best be implemented either during or after the TurboJPEG v3 API overhaul. (IOW, this enhancement would have a dependency of TurboJPEG 3 API overhaul #517.)
  • This enhancement, as it would need to be implemented, would not address your use case unless tjInit*() could be refactored in such a way that caller-supplied memory was used for the opaque structure. In the context of the TurboJPEG v3 API overhaul, I will ponder whether such a refactor of tjInit*() makes sense.

@eloparco
Copy link

eloparco commented Jun 9, 2023

The architecture of the libjpeg API library is such that the memory manager implementation can't be replaced at run time.

Hello, is this still the case? In my use case, I want to pass a custom allocator (that would do an allocation in wasm memory)

@dcommander
Copy link
Member

Hello, is this still the case? In my use case, I want to pass a custom allocator (that would do an allocation in wasm memory)

Yes. If you're using the libjpeg API, then you'll need to modify the library source code in order to use allocate/free methods other than malloc() and free(). But doesn't WASM emulate malloc() and free()? I have successfully built and tested libjpeg-turbo with WASM. Functionally, the only issue was the lack of SIMD extensions, i.e. low performance.

@eloparco
Copy link

eloparco commented Jun 9, 2023

Thanks for the reply. In my use case, I want to call the libjpeg API from native and pass an allocator that would allocate in WASM memory. Instead, what you were referring to seems more like compiling libjpeg to WASM.

@dcommander
Copy link
Member

Your path of least resistance there would be to modify jmemnobs.c.

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

No branches or pull requests

3 participants