[OpenJDK 2D-Dev] Question regarding the integration of harfbuzz (JEP 258)

Thu Dec 10 16:45:32 UTC 2015

Thanks Volker!

On Thu, Dec 10, 2015 at 5:12 PM, Volker Simonis <volker.simonis at gmail.com>
wrote:

> And now I see that you've already pushed the change! Thanks a lot. I'd
> only like to suggest the following minor enhancement to my initial change.
> It restricts the change to the case where HarfBuzz will be compiled with
> IBM's xlC compiler because it uses xlC compiler intrinsics. I know there
> are people out there who package open source software for AIX but they
> usually compile the whole toolchain with GCC. This minor addition will
> protect them from running into compile errors if they should ever try to
> compile HarfBuzz with GCC on AIX.
>

Pushed up!  Though, users with gcc probably will have
the HAVE_INTEL_ATOMIC_PRIMITIVES branch kick in and the AIX branch not
exercised.  But the fix is good so pushed.

Cheers,
behdad

> It would be great if you could add that as well to the upstream version.
>
> I will then update my OpenJDK patch to contain exactly your "shuffeled"
> upstream version so we don't get any problems when merging from upstream in
> the future.
>
> Thanks a lot once again,
> Volker
>
>
> diff -r fd95f33023e9
> src/java.desktop/share/native/libfontmanager/harfbuzz/hb-atomic-private.hh
> ---
> a/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-atomic-private.hh
> Thu Dec 10 12:13:22 2015 +0100
> +++
> b/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-atomic-private.hh
> Thu Dec 10 16:55:58 2015 +0100
> @@ -119,7 +119,7 @@
>  #define hb_atomic_ptr_impl_cmpexch(P,O,N)       ( ({__machine_rw_barrier
> ();}), atomic_cas_ptr ((void **) (P), (void *) (O), (void *) (N)) == (void
> *) (O) ? true : false)
>
>
> -#elif !defined(HB_NO_MT) && defined(_AIX)
> +#elif !defined(HB_NO_MT) && defined(_AIX) && defined(__IBMCPP__)
>
>  #include <builtins.h>
>
>
>
> On Thu, Dec 10, 2015 at 3:55 PM, Behdad Esfahbod <behdad at google.com>
> wrote:
>
>> Slightly shuffled patch was committed upstream:
>>
>> https://github.com/behdad/harfbuzz/commit/70b33edae7c8b9c031b83c95f00cb383789f1041
>>
>> Thanks,
>> behdad
>>
>>
>> On Thu, Dec 10, 2015 at 2:36 PM, Behdad Esfahbod <behdad at google.com>
>> wrote:
>>
>>> Hi Volker, Steven,
>>>
>>> I'd happily take AIX atomic primitive patch for HarfBuzz.  Yes, github
>>> Pull-Requests are fine.  As you correctly identified, HarfBuzz uses atomic
>>> ops and mutexes to synchronize access to global data as well as
>>> reference-counting from multiple threads, even though it doesn't uses
>>> threads itself.
>>>
>>> Does AIX use pthread for threading?  I can definitely add a threading
>>> stress-test to HarfBuzz.  I definitely should.  Filed here:
>>>
>>>   https://github.com/behdad/harfbuzz/issues/195
>>>
>>> Thanks,
>>> behdad
>>>
>>> On Thu, Dec 10, 2015 at 2:27 PM, Volker Simonis <
>>> volker.simonis at gmail.com> wrote:
>>>
>>>> Hi Steven,
>>>>
>>>> thanks for your response.
>>>>
>>>> On Thu, Dec 10, 2015 at 1:44 AM, Steven R. Loomis <srl at icu-project.org>
>>>> wrote:
>>>>
>>>>> Volker,
>>>>>  0. I’d like to see what the crashing stack frame is when NOT on
>>>>> harfbuzz, because there should be no change.
>>>>>
>>>>>
>>>> This was another problem. Please see my previous mail in this thread
>>>> where I've described the cause in more detail.
>>>>
>>>>
>>>>>  1. Phil addressed this one
>>>>>
>>>>>  2. I will take a look on AIX. I’ll see if I can build Harfbuzz itself
>>>>> on AIX at first.
>>>>>
>>>>>
>>>> I've fixed it now and just send around a request for review:
>>>> http://mail.openjdk.java.net/pipermail/2d-dev/2015-December/006065.html
>>>>
>>>> The mail contains a link to the bug and the patch which fixes it.
>>>>
>>>> I also want to contribute this to harfbuzz. Is it OK to send a pull
>>>> request to https://github.com/behdad/harfbuzz ?
>>>>
>>>>  3. Yes, I’ll work on putting some docs together for that test.
>>>>> Actually, other tests such as the Font2DTest  would be a good test. Or, any
>>>>> text in a complex script such as Hindi.
>>>>>
>>>>>
>>>> That would be great. For now, the Font2DTest runs without a problem
>>>> with harfbuzz on AIX (and linux/ppc64).
>>>>
>>>>
>>>>>  4. I’m not sure. A threaded stress test would be good.  Perhaps
>>>>> Harfbuzz-in-JDK could leverage the
>>>>> hotspot/src/os_cpu/../vm/atomic_os_cp.inline.hpp you mentioned.
>>>>>
>>>>>  Behdad - file is here
>>>>> http://hg.openjdk.java.net/jdk9/client/hotspot/file/c8e212fb27d0/src/os_cpu/linux_x86/vm/atomic_linux_x86.inline.hpp
>>>>>  - any comments on this or other items?
>>>>>
>>>>>
>>>> That sounds like a good idea but unfortunately the hotspot primitives
>>>> are in a different repository (source-code wise) and in a different library
>>>> (i.e. libjvm.so) which doesn't export them. So the best we can do for now
>>>> would be to copy the missing parts over to the jdk repository (and that's
>>>> actually what I've done now for the AIX version of the primitives).
>>>>
>>>> Regards,
>>>> Volker
>>>>
>>>>
>>>>> -s
>>>>>
>>>>>
>>>>> -------- Original Message -------- Subject: Re: [OpenJDK 2D-Dev]
>>>>> Question regarding the integration of harfbuzz (JEP 258) Date: Wed,
>>>>> 09 Dec 2015 09:02:33 -0800 From: Philip Race <philip.race at oracle.com>
>>>>> <philip.race at oracle.com> Organization: Oracle Corporation To:
>>>>> 2d-dev at openjdk.java.net
>>>>>
>>>>>
>>>>> Hi Volker,
>>>>>
>>>>> Running with ICU should stop harfbuzz use completely so if
>>>>> you are still seeing a crash, perhaps it is due to something else entirely.
>>>>> Especially since a "simple AWT program" should not load layout anyway.
>>>>> There was a fair amount of new and changed code pushed recently ...
>>>>>
>>>>> Yes, we need to run harfbuzz in multiple concurrent threads in the JDK.
>>>>> A single threaded lock on harfbuzz would throttle text operations.
>>>>> I suppose it is possible that we do not strictly need it as we currently
>>>>> create a
>>>>> new harfbuzz "instance" each time we go to invoke layout but that
>>>>> is something that at least theoretically could change.
>>>>>
>>>>> The approach to synchronization is all from up-stream harfbuzz.
>>>>> The harfbuzz mailing list might be the place to discuss that.
>>>>> I'd expect it does work on AIX as a principal driver here is that
>>>>> the IBM sponsored ICU project has deprecated its layout engine
>>>>> and IBM/ICU are moving to harfbuzz so I'd be somewhat astonished
>>>>> if they haven't yet tried it on AIX. Did you try just getting a copy
>>>>> of the full hotspot library and running its configure on AIX to see if
>>>>> harfbuzz can work out which options it wants there ? Obviously
>>>>> I did not try that since I don't have AIX. Steven Loomis of IBM
>>>>> who submitted the JEP should be able to help with a lot of this.
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 12/8/15, 10:50 AM, Volker Simonis wrote:
>>>>> > Hi,
>>>>> >
>>>>> > the integration of harfbuzz broke our AIX build because there's no
>>>>> > implementation available for the hb_atomic macros in
>>>>> > hb-atomic-private.hh. I've fixed that locally but still get a crash
>>>>> > when running a simple AWT program (even with
>>>>> > -Dsun.font.layoutengine=icu).
>>>>> >
>>>>> > I'm curretnly debugging the problem, but I have some general questions:
>>>>> >
>>>>> > 1. Do we really need the multi-threaded features of harfbuzz in OpenJDK?
>>>>> >
>>>>> > 2. Why does hurbuzz require these synchronization macros? From a first
>>>>> > glance it doesn't seem to use multiple threads by itself. Does it have
>>>>> > some global data structures which it needs to protect if harfbuzz is
>>>>> > called from various threads?
>>>>> >
>>>>> > 3. Is there a way to test the hurfbuzz engine on a new platform? I saw
>>>>> > the new test TestLayoutVsICU.java but it is marked as "manual" and it
>>>>> > seems to require external fonts (at least some of which seem to be
>>>>> > freely available). Could you please provide at least a minimal
>>>>> > description on how this test can be run?
>>>>> >
>>>>> > 4. Is there a way to test that the implementation of the hb_atomic
>>>>> > macros in hb-atomic-private.hh is correct (i.e. a way to stress
>>>>> > harfbuzz from multiple threads)? I think this is important as I
>>>>> > couldn't find a good description of the requirements for the atomic
>>>>> > macros and on weak memory model architectures we should really get
>>>>> > this right. For example there's no description of which fences are
>>>>> > required around the various atomic operations. Getting this right
>>>>> > across all platforms in the HotSpot (see
>>>>> > hotspot/src/os_cpu/../vm/atomic_os_cp.inline.hpp) is already hard
>>>>> > enough so I'm a little concerned that we are now introducing similar
>>>>> > primitives in the class library.
>>>>> >
>>>>> > Thank you and best regards,
>>>>> > Volker
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20151210/a80b8f59/attachment.html>