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

Thu Dec 10 14:55:48 UTC 2015

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
>>>
>>>
>>>
>>
>