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
