Hmm. Should JEP 254 really have " removed the handling of empty strings from jni_GetStringCritical()."
without tracking down uses and updating them ?

I didn't mean IBM already ship harfbuzz in their JDK but I thought
they were already using it in some other IBM products.

-phil.

On 12/9/15, 10:37 AM, Volker Simonis wrote:
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.r...@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

Reply via email to