On 2018-03-23 18:24, Volker Simonis wrote:
Hi Magnus,

thanks for addressing this long standing issue! I haven't looked at
the changes, but just want to share some general and historical notes:

- Compiling with "-fvisibility=hidden" which hides all symbols expect
the ones explicitly exported with
"__attribute__((visibility("default")))" has been requested by SAP
back in 2007 even before we had OpenJDK (see "Use -fvisibility=hidden
for gcc compiles" https://bugs.openjdk.java.net/browse/JDK-6588413)
and finally pushed into the OpenJKD around 2010.
- "-fvisibility=hidden" gave us performance improvements of about 5%
(JBB2005) and 2% (JVM98) on Linux/IA64 and 1,5% (JBB2005) and 0,5%
(JVM98) on Linux/PPC64 because the compiler could use faster calls for
non exported symbols. This improvement was only very small on x86
tough.
That's a nice side effect! Although my main purpose here is maintainability, gaining performance is nothing I say no to. :)
- "-fvisibility=hidden"/"__attribute__((visibility("default")))"
applies BEFORE using the map files in the linking step (i.e. hidden
symbols can't be exported any more even if mentioned in the map file)
- because of the performance improvements we got by using
"-fvisibility=hidden" it was worth while using it even though we had
the mapfiles at the end of the process.

Then we had several mail threads (which you probably remember because
you were involved :) where we discussed to either remove the map files
completely or instead generate them automatically during the build:

http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-February/thread.html#12412
http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-February/thread.html#12628

The main arguments against removing the map files at that time were:

1. the danger to re-export all symbols of statically linked libraries
(notably libstdc++ at that time)
2. loosing exports of compiler generated symbols like vtables which
are required by the Serviceability Agent

Point 1 is not a problem today, because I don't think we do any static
linking any more. If we still do it under some circumstances, this
problem should be re-evaluated.
Well, we do static linking with libstdc++ on linux, in certain circumstances. See "--with-stdc++lib=<static>,<dynamic>,<default>". Fortunately, this is not a problem. The linker can be told not to include symbols from statically linked libraries, which is exactly what I do with LDFLAGS_JDKLIB += -Wl,--exclude-libs,ALL.

The corresponding feature does not exist for the solstudio linker, but fortunately we do not use statically linked libraries there.

Point 2 is only relevant for HotSpot. But because of "8034065: GCC 4.3
and later doesn't export vtable symbols any more which seem to be
needed by SA" (https://bugs.openjdk.java.net/browse/JDK-8034065),
exporting such symbols trough a map files doesn't work any more
anyway. So this isn't a problem either.
In any case, that's a question for another day. :) There were reasons I left Hotspot out of this fix, and the question about the SA agent is one of them. :) As you say, I think they do not apply anymore, but I'll return to consider Hotspot later on.

So to cut a long story short - I think the time is ripe to get rid of
the map files. Thumbs up from me (meant as moral support, not as a
concrete review :)
Thanks for the kind words!

/Magnus


Regards,
Volker

On Fri, Mar 23, 2018 at 5:05 PM, mandy chung <mandy.ch...@oracle.com> wrote:
This is a very good change and no more mapfile to maintain!!

Please do file JBS issues for the component teams to clean up their exports.

Mandy


On 3/23/18 7:30 AM, Erik Joelsson wrote:
I have looked at the build changes and they look good.

Will you file followups for each component team to look over their
exported symbols, at least for the libraries with $(EXPORT_ALL_SYMBOLS)? It
sure looks like there is some technical debt laying around here.

/Erik


On 2018-03-23 06:56, Magnus Ihse Bursie wrote:
With modern compilers, we can use compiler directives (such as
_attribute__((visibility("default"))), or __declspec(dllexport)) to control
symbol visibility, directly in the source code. This has historically not
been present on all compilers, so we had to resort to using mapfiles (also
known as linker scripts).

This is no longer the case. Now all compilers we use support symbol
visibility directives, in one form or another. We should start using this.
Since this has been the only way to control symbol visibility on Windows,
for most of the shared code, we already have proper JNIEXPORT decorations in
place.

If we fix the remaining platform-specific files to have proper JNIEXPORT
tagging, then we can finally get rid of mapfiles.

This fix removed mapfiles for all JDK libraries. It does not touch
hotspot libraries nor JDK executables; they will have to wait for a future
fix -- this was complex enough. This change will not have any impact on
macosx, since we do not use mapfiles there, but instead export all symbols.
(This is not a good idea, but I'll address that separately.) This change
will also have a minimal impact on Windows. The only reason Windows is
impacted at all, is that some changes needed by Solaris and Linux were
simpler to fix for all platforms.

I have strived for this change to have no impact on the actual generated
code. Unfortunately, this was not possible to fully achieve. I do not
believe that these changes will have any actual impact on the product,
though. I will present the differences more in detail further down. Those
who are not interested can probably skip that.

The patch has passed tier1 testing and is currently running tier2 and
tier3. Since the running code is more or less (see caveat below) unmodified,
I don't expect any testing issues.

Bug: https://bugs.openjdk.java.net/browse/JDK-8200178
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01

Details on changes:
Most of the source code changes are (unsurprisingly) in java.base and
java.desktop. Remaining changes are in jdk.crypto.ucrypto,
jdk.hotspot.agent, jdk.jdi and jdk.jdwp.agent.

Source code changes does almost to 100% consists in decorating an
exported function with JNIEXPORT. I have also followed the long-standing
convention of adding JNICALL. This is a no-op on non-Windows platforms, so
for most of the changes this is purely cosmetic (and possibly adding in
robustness, should the function ever be used on Windows in the future). I
have also followed the stylistic convention of putting "JNIEXPORT <return
type> JNICALL" on a separate line. For some functions, however, this might
cause a change in calling convention on Windows. Since this can not apply to
exported functions on Windows (otherwise they would already have had
JNIEXPORT), I do not think this matters anything.

A few libraries did not have a mapfile, on Linux and/or Solaris. This
actually meant that all symbols were exported. It is highly unclear if this
was known and intended by the original make rule writer. I have emulated
this by adding the flag $(EXPORT_ALL_SYMBOLS) to these libraries. Hopefully,
we can remove this flag and fix proper exported symbols in the future.

I have run the complete build using COMPARE_BUILD, and made a thourough
analysis of the differences for Linux and Solaris. All native libraries have
symbol differences, but most of them are trivial and/or harmless. As a
result, most libraries have disasm differences as well, but these too seem
trivial and harmless. The differences in symbols that are common to all
libraries include:
  * Internal symbols such as __bss_start, _edata, _end and _fini are now
global. (They are imported as such from the compiler libraries/archives, and
we have no linker script to override this behavior).
  * The versioning tag SUNWprivate_1.1 is not included, and thus neither
the .gnu.version_d symbol.
  * There are a few differences in the symbol and/or mangling of some
local functions. I'm not sure what's causing this,
but it's unlikely to have any effect on the product.

Another common source for change in symbols is due to previous platform
differences. For instance, if we had "JNIEXPORT int JNICALL do_foo() { ...
}", but do_foo was not in the mapfile, the symbol was exported on Windows
but not on Linux and Solaris. (Presumable since it was not needed there,
even though it was compiled for those platforms as well.) Now, with the
mapfiles gone, do_foo() will be exported on all platforms. And contrary,
functions that are compiled on all platforms, and were exported in mapfiles,
but now have gotten an JNIEXPORT decoration, will now be visible even on
Windows. (This accounts for half of the noticed symbol differences on
Windows.) I could have made the JNIEXPORT conditional on OS, but I didn't
think the mess in source code were worth the keeping of binary confidence
with the old build.

A third common source for change in symbols is due to exported functions
"leaking" across library borders. For instance, some functions in
java.desktop is compiled in both libawt_xawt and libawt_headless, but they
were previously only included in the mapfile for one of these libraries.
Now, since the visibility is determined by the source code itself, it gets
exported in both libraries. A variant of this is when a library depends on
another JDK library, and includes the header file from that other library,
which in turn declares a function as JNIEXPORT. This will cause the
including library to also export the function. This accounts for the other
half of the changes on Windows. A typical example of this is that multiple
libraries now re-export hotspot symbols from libjvm.so, like jio_fprintf. (I
have not listed the libjvm re-exports below.)

Note that  Java_java_io_FileOutputStream_close0 in
java.base/unix/native/libjava/FileOutputStream_md.c is no longer exported,
and can probably be removed.

Here is a detailed table showing and accounting for all the remaining
differences found on Linux and Solaris:
java.base/unix/native/libjava: Java_java_io_FileOutputStream_close0 is
now also exported on unix platforms due to JNIEXPORT.

java.base/jspawnlauncher: On solaris, we also include
libjava/childproc.o, which
now exports less functions than it used to (it used to export all
functions, now it is compiled with visibility=hidden).

java.base/java(w).exe: Is now also exporting the following symbols due to
added JNIEXPORT in libjli on Windows:
(Yes, executables can export symbols on Windows. Confusing, I know.)
  JLI_AddArgsFromEnvVar
  JLI_CmdToArgs
  JLI_GetAppArgIndex
  JLI_GetStdArgc
  JLI_GetStdArgs
  JLI_InitArgProcessing
  JLI_Launch
  JLI_List_add
  JLI_List_new
  JLI_ManifestIterate
  JLI_MemAlloc
  JLI_MemFree
  JLI_PreprocessArg
  JLI_ReportErrorMessage
  JLI_ReportErrorMessageSys
  JLI_ReportExceptionDescription
  JLI_ReportMessage
  JLI_SetTraceLauncher
  JLI_StringDup

java.desktop:/libawt_xawt: The following symbols are now also exported on
linux and solaris due to JNIEXPORT:
  awt_DrawingSurface_FreeDrawingSurfaceInfo
  awt_DrawingSurface_GetDrawingSurfaceInfo
  awt_DrawingSurface_Lock
  awt_DrawingSurface_Unlock
  awt_GetColor

The following symbols are now also exported on linux and solaris due to
JNIEXPORT (they were previously
  exported only in libawt):
  Java_sun_awt_DebugSettings_setCTracingOn__Z
  Java_sun_awt_DebugSettings_setCTracingOn__ZLjava_lang_String_2
  Java_sun_awt_DebugSettings_setCTracingOn__ZLjava_lang_String_2I
  Java_sun_awt_X11GraphicsConfig_getNumColors

java.desktop:/libawt_headless: The following symbols are now also
exported due to JNIEXPORT (they were previously
  exported only in libawt_xawt and/or libawt):
  Java_sun_java2d_opengl_GLXGraphicsConfig_getGLXConfigInfo
  Java_sun_java2d_opengl_GLXGraphicsConfig_getOGLCapabilities
  Java_sun_java2d_x11_X11PMBlitLoops_updateBitmask
  Java_sun_java2d_x11_X11SurfaceData_isShmPMAvailable
  X11SurfaceData_GetOps

java.desktop/libawt: The following symbols are now also exported on
Windows, due to added
JNIEXPORT:
  SurfaceData_InitOps
  mul8table
  div8table
  doDrawPath
  doFillPath
  g_CMpDataID
  initInverseGrayLut
  make_dither_arrays
  make_uns_ordered_dither_array
  path2DFloatCoordsID
  path2DNumTypesID
  path2DTypesID
  path2DWindingRuleID
  sg2dStrokeHintID
  std_img_oda_blue
  std_img_oda_green
  std_img_oda_red
  std_odas_computed
  sunHints_INTVAL_STROKE_PURE

java.desktop/libawt on solaris:
A number of "#pragma weak" directives was previously overridden by the
mapfile.
Now these directives are respected, so these symbols are now weak instead
of local:
  ByteGrayToIntArgbPreConvert_F
  ByteGrayToIntArgbPreScaleConvert_F
  IntArgbBmToFourByteAbgrPreScaleXparOver_F
  IntArgbToIntRgbXorBlit_F
  IntBgrToIntBgrAlphaMaskBlit_F

java.desktop/libawt on solaris: These are now also exported due to
JNIEXPORT in libmlib_image.
  j2d_mlib_ImageCreate
  j2d_mlib_ImageCreateStruct
  j2d_mlib_ImageDelete

java.desktop/libawt on solaris: This is now also exported due to
JNIEXPORT:
  GrPrim_CompGetXorColor
  SurfaceData_GetOpsNoSetup
  SurfaceData_IntersectBoundsXYWH
  SurfaceData_SetOps
  Transform_GetInfo
  Transform_transform

java.desktop/libsplashscreen: JNI_OnLoad is now exported on linux and
solaris due to JNIEXPORT.
libspashscreen also had JNIEXPORT (actually a pure _declspec(dllexport))
but no JNICALL, which I added as
a part of converting to JNIEXPORT. The same goes for libmlib_image .

jdk.sctp/libsctp: handleSocketError is now exported on linux and solaris
due to JNIEXPORT in libnio.

java.instrument:/libinstrument: Agent_OnUnload is now also exported on
linux and solaris platforms due to JNIEXPORT.
JLI_ManifestIterate is now also exported on Windows, due to added
JNIEXPORT in libjli.

jdk.management/libmanagement_ext:
Java_com_sun_management_internal_Flag_setDoubleValue is now also exported on
linux and solaris platforms due to JNIEXPORT.

/Magnus



Reply via email to