[HarfBuzz] To HB_NO_MT or not to HB_NO_MT

[Behdad Esfahbod]

On 15-03-28 03:28 PM, Konstantin Ritt wrote:
> Hi list,
> 
> Intro: the configure script tries to detect the Intel and Solaris atomic
> primitives/ops support and if that failed, HB falls back to the thread-unsafe
> default implementation and warns the user (and also proposes to suppress the
> warning by defining HB_NO_MT ;) ). Moreover, HB_NO_MT is the only alternative
> suggested by Android.mk (though docs/ANDROID-ATOMICS.htm says the atomics are
> supported)!

The Android.mk in the repo has two purposes:

  1. Initially it was requested by people who wanted to build Android for
their own app, even though the can use the autoconf build system with Android;
perhaps they are not very proficient in the latter,

  2. For building within Android itself.  Since HarfBuzz is only used by
Minikin, which has its own locking, we don't need threadsafe HB in Android.


> Issue: in a multi-threaded application, there is no possibility to check if HB
> library we're linking to is thread-safe -> we blindly assume it is.

Right.  I think a shared-library version of HB must always be threadsafe, and
if it's not, that's a packaging bug.

The biggest user of no-MT is Firefox, which AFAIU always uses HB from one
thread and doesn't want to incur the MT cost.


> Proposed solution: in case the configure script failed to detect the supported
> atomic primitives, proceed with a warning and expect the build to fail if the
> user didn't provide his own implementation (ie. in config.h or via some
> external include). This way, we could even get rid of HB_NO_MT path -- where
> explicitly required, the user would be able to provide his no-MT
> implementation (ie. for HB built in a single-threaded application).
> 
> WDYT?

I would like to say I'm fine with that; however, the single-threaded
implementation is not exactly trivial, so I don't like to leave it to
individuals to figure it out.

How about this:

For both warnings in hb-warnings.cc, that is, the MT warning and the
no-Unicode-funcs warnings, make both into hard errors.  For the former, remove
mentioning HB_NO_MT, and instead suggest defining correct platform flags.  For
the latter, suggest including hb-ucdn into their build.

WDYT?

b