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

[Volker Simonis]

Hi Phil,

thanks a lot for your answer. Please find my comments inline:

On Wed, Dec 9, 2015 at 6:02 PM, Philip Race <philip.race at oracle.com> wrote:
> 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.

Yes, it was indeed something completely different. It was "8145015:
jni_GetStringCritical asserts for empty strings"
(https://bugs.openjdk.java.net/browse/JDK-8145015) which I've opened
(and fixed :) today. The problem was that
sun.awt.X11.XlibWrapper.XSetLocaleModifiers() was called with an empty
string from sun.awt.X11.XToolkit.init() which crashed in the debug
build for the ISO8859-1 locale:

V  [libjvm.so+0xffffffff]  VMError::report_and_die(Thread*,const
char*,int,const char*,const char*,char*)+0x68
V  [libjvm.so+0xffffffff]  report_vm_error(const char*,int,const
char*,const char*,...)+0x8c
V  [libjvm.so+0xffffffff]  typeArrayOopDesc::char_at_addr(int) const+0x74
V  [libjvm.so+0xffffffff]  jni_GetStringCritical+0x2d4
C  [libjava.so+0xffffffff]  getString8859_1Chars+0x70
C  [libjava.so+0xffffffff]  JNU_GetStringPlatformChars+0xc4
C  [libawt_xawt.so+0xffffffff]
Java_sun_awt_X11_XlibWrapper_XSetLocaleModifiers+0x4c
j  sun.awt.X11.XlibWrapper.XSetLocaleModifiers(Ljava/lang/String;)Ljava/lang/String;+0
j  sun.awt.X11.XToolkit.init()V+9
j  sun.awt.X11.XToolkit.<init>()V+97

During debugging I also found out that "ISO-8859-1", which is the name
this locale has on Linux, isn't correctly detected as 'FAST_8859_1'
encoding. That's probably why nobody saw this error before. And that's
why it's always good to double check the code on AIX :)

I'll open a bug for the encoding problem on Linux tomorrow...

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

That's strange, because I've looked at the up-stream harfbuzz project
and they definitely don't have the atomic instructions for AIX.

So either IBM completely dropped AIX or they are only using it from a
single thread or they haven't contributed their changes back to the
OpenJDK - which would be sad :( So it would be definitely interesting
to hear the actual internals from Steven :)

That said, I've now implemented the three atomics myself by using the
same compiler intrinsics like in the hotspot. I'll post a webrev
tomorrow...

Regards,
Volker

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