<div dir="ltr">Just tested this against another test app I have (not mine, a demo app that's written in C) and it failed to link as it was looking for the C++ named-mangled versions of those externed functions. They should probably be declared with extern "C" around them so that they can be defined by C applications.<div> </div><div>In hindsight I should have mentioned this before, however I'd always assumed HarfBuzz was a C library due to the way its API is designed, but the templates in the OT font code proves otherwise :)</div><div> </div><div>-Jamie.</div></div><div class="gmail_extra"> <div class="gmail_quote">On 5 October 2015 at 19:58, Jamie Dale <<a href="mailto:jamiedale88+harfbuzz@gmail.com" target="_blank">jamiedale88+harfbuzz@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Just tested this and it worked fine. Thanks :)</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"> <div class="gmail_quote">On 3 October 2015 at 13:20, Behdad Esfahbod <<a href="mailto:behdad.esfahbod@gmail.com" target="_blank">behdad.esfahbod@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 15-10-02 10:00 PM, Jamie Dale wrote: > That's similar to what I originally had, so I'm fine with something like that. > My only question is, what would hb_malloc_impl, hb_calloc_impl, > hb_realloc_impl, and hb_free_impl be defined to in your example? Define it to the name of your external function. I don't want to fix a name in HarfBuzz itself. Updated patch to add externs: diff --git a/src/hb-private.hh b/src/hb-private.hh index 07550cb..826e41a 100644 --- a/src/hb-private.hh +++ b/src/hb-private.hh @@ -54,6 +54,23 @@ #include <stdarg.h> +/* Compile-time custom allocator support. */ + +#if defined(hb_malloc_impl) \ + && defined(hb_calloc_impl) \ + && defined(hb_realloc_impl) \ + && defined(hb_free_impl) +extern void* hb_malloc_impl(size_t size); +extern void* hb_calloc_impl(size_t nmemb, size_t size); +extern void* hb_realloc_impl(void *, size_t size); +extern void hb_free_impl(void *ptr); +#define malloc hb_malloc_impl +#define calloc hb_calloc_impl +#define realloc hb_realloc_impl +#define free hb_free_impl +#endif + I'll go ahead and commit this. Let me know if it doesn't work. > I went with the extern function approach so I could inject my own allocator > functions into HarfBuzz at link time, rather than have to include any of our > code into HarfBuzz at compile time (since any local code changes we make have > to be re-applied against new versions). > > For your reference, this was my original implementation: > > /* Override the allocation functions when > * HAVE_EXTERNAL_ALLOCATOR is defined. */ > #ifdef HAVE_EXTERNAL_ALLOCATOR > extern void* _hb_malloc(size_t); > extern void* _hb_calloc(size_t, size_t); > extern void* _hb_realloc(void*, size_t); > extern void _hb_free(void*); > #define malloc _hb_malloc > #define calloc _hb_calloc > #define realloc _hb_realloc > #define free _hb_free > #endif > > Failure to define those functions in something that linked to HarfBuzz would > produce an unresolved external symbol error at link time. > > -Jamie. > > On 2 October 2015 at 08:08, Behdad Esfahbod <<a href="mailto:behdad.esfahbod@gmail.com" target="_blank">behdad.esfahbod@gmail.com</a> <div><div>> <mailto:<a href="mailto:behdad.esfahbod@gmail.com" target="_blank">behdad.esfahbod@gmail.com</a>>> wrote: > > Hi Jamie, > > I'm opposed to runtime custom allocators, but am fine with something like what > you suggest. However, that has two down sides: > > 1. It touches a lot of callsites, > > 2. I have to add macros to make sure stock malloc etc are not used by > mistake. > > If doing 2., I might as well just define them to their custom behavior. Ie, > would this patch work for you: > > diff --git a/src/hb-private.hh b/src/hb-private.hh > index 07550cb..167c067 100644 > --- a/src/hb-private.hh > +++ b/src/hb-private.hh > @@ -54,6 +54,19 @@ > #include <stdarg.h> > > > +/* Compile-time custom allocator support. */ > + > +#if defined(hb_malloc_impl) \ > + && defined(hb_calloc_impl) \ > + && defined(hb_realloc_impl) \ > + && defined(hb_free_impl) > +#define malloc hb_malloc_impl > +#define calloc hb_calloc_impl > +#define realloc hb_realloc_impl > +#define free hb_free_impl > +#endif > + > + > /* Compiler attributes */ > > > > > On 15-09-30 03:49 PM, Jamie Dale wrote: > > Since there was no movement here I went ahead and hacked in our allocator by > > redefining malloc, calloc, realloc, and free to point to extern'd functions > > that I define in our application. This yielded some performance benefits for > > us, so I'm hoping that a more standard HarfBuzz solution might appear at > some > > point :) > > > > If you're not keen on the idea of allowing the allocator to be swapped > out at > > runtime, would you at least allow it to be extern'd at compile time? I'm not > > familiar with HarfBuzz coding standards, but I'd propose adding > something like > > a HAVE_EXTERNAL_ALLOCATOR define to control this, and then define your own > > allocator functions that you'd use in place of malloc, etc. > > > > Basically, something like this (I currently have this code in > hb-private.hh): > > > > /* Define our allocation functions. These may be provided by an > > * external source when HAVE_EXTERNAL_ALLOCATOR is defined. */ > > #ifdef HAVE_EXTERNAL_ALLOCATOR > > extern void* _hb_malloc(size_t); > > extern void* _hb_calloc(size_t, size_t); > > extern void* _hb_realloc(void*, size_t); > > extern void _hb_free(void*); > > #define hb_malloc _hb_malloc > > #define hb_calloc _hb_calloc > > #define hb_realloc _hb_realloc > > #define hb_free _hb_free > > #else > > #define hb_malloc malloc > > #define hb_calloc calloc > > #define hb_realloc realloc > > #define hb_free free > > #endif > > > > You'd then replace calls to malloc, calloc, realloc, and free from within > > HarfBuzz to use hb_malloc, hb_calloc, hb_realloc, and hb_free. > > > > This is basically the code I already have, except I > > redefined malloc, calloc, realloc, and free rather than create the > > hb_*variants, as I didn't want to change too much HarfBuzz code. > > > > Thoughts? > > </div></div>> > On 31 August 2015 at 22:17, Jamie Dale <<a href="mailto:jamiedale88%2Bharfbuzz@gmail.com" target="_blank">jamiedale88+harfbuzz@gmail.com</a> <mailto:<a href="mailto:jamiedale88%252Bharfbuzz@gmail.com" target="_blank">jamiedale88%2Bharfbuzz@gmail.com</a>> > > <mailto:<a href="mailto:jamiedale88%2Bharfbuzz@gmail.com" target="_blank">jamiedale88+harfbuzz@gmail.com</a> <mailto:<a href="mailto:jamiedale88%252Bharfbuzz@gmail.com" target="_blank">jamiedale88%2Bharfbuzz@gmail.com</a>>>> > wrote: > > > > Hey, > > > > I'm not sure if this is correct place to post/discuss feature requests... > > hopefully it is. > > > > Our project makes use of a custom memory allocator, and we like to ensure > > that our third-party libraries are using this where possible, as our > > allocator can offer significantly better performance, and can also allow > > us better memory profiling and debugging support. > > > > A look through the HarfBuzz source code would suggest that it doesn't > > currently support custom memory allocators, and instead just directly > > calls the standard malloc/free/etc functions. I was wondering if you'd > > ever considered (or would consider) adding a way to override this behaviour? > > > > In my case, I'd just need a single global override to be set before my > > first call to any other HarfBuzz code... basically something like > > the FT_Memory struct from FreeType: > > <a href="http://www.freetype.org/freetype2/docs/reference/ft2-system_interface.html#FT_Memory" rel="noreferrer" target="_blank">http://www.freetype.org/freetype2/docs/reference/ft2-system_interface.html#FT_Memory</a> > > > > Thanks, > > Jamie. > > > > > > > > > > _______________________________________________ > > HarfBuzz mailing list > > <a href="mailto:HarfBuzz@lists.freedesktop.org" target="_blank">HarfBuzz@lists.freedesktop.org</a> <mailto:<a href="mailto:HarfBuzz@lists.freedesktop.org" target="_blank">HarfBuzz@lists.freedesktop.org</a>> > > <a href="http://lists.freedesktop.org/mailman/listinfo/harfbuzz" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/harfbuzz</a> > > > > </blockquote></div> </div> </div></div></blockquote></div> </div>