RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC
Kim Barrett
kim.barrett at oracle.com
Tue Apr 2 23:02:06 UTC 2019
> On Apr 2, 2019, at 5:39 PM, Erik Joelsson <erik.joelsson at oracle.com> wrote:
>
> In JDK-8204551, exceptions.hpp started using THIS_FILE instead of __FILE__ to generate exception messages. This is causing the precompiled header to no longer provide any benefit on Linux/GCC. The problem is that the THIS_FILE macro is provided on the command line and is different for each compile unit. Because of this, it seems GCC invalidates the precompiled header completely. (On other platforms, the value of THIS_FILE is instead ignored).
>
> The goal of JDK-8204551 was to shorten the path to the file where an exception was raised to avoid truncation. This patch provides a different approach to achieve this, that also fixes the issue for other platforms and compilers. It also fixes the performance issue with precompiled headers.
>
> For Hotspot (at least for now), we stop setting -DTHIS_FILE on the compiler command line completely. Instead this macro is defined in macros.hpp, based on __FILE__ but with an offest. This offset is calculated in configure. For newer versions of GCC (8+) there is a new flag, -fmacro-prefix-map=, which can be used to rewrite the __FILE__ macro. Configure checks if this flag is available and uses it instead of the offset macro if possible.
>
> I only touched the one usage of THIS_FILE/__FILE__ in this patch. It's possible we should rewrite all references to __FILE__ to improve debug/error.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8221851
>
> Webrev: http://cr.openjdk.java.net/~erikj/8221851/webrev.01/index.html
>
> /Erik
Here's an alternative approach that seems simpler. Don't pass
THIS_FILE to HotSpot code (or ignore it if that works). Instead add
inline const char* this_file_helper(const char* s) {
const char* result = strrchr(s, '/');
return (result == NULL) ? s : (result + 1);
}
and in the one place where HotSpot is using THIS_FILE
(exceptions.hpp), instead use
this_file_helper(__FILE__)
(This assumes the path separator is '/', but there's probably a
constant for that somewhere if it's needed.)
(this_file_helper() is only needed to deal with the case where
__FILE__ contains no path separators; that might not even be an issue,
in which case it can be dropped and just add 1 to the strrchr result.
Also not wedded to that name; this is more or less basename(3).)
Empirically, gcc will optimize that all quite nicely. I don't know
about other platforms, but the worst that happens is we take a small
runtime hit when "throwing" an exception.
This doesn't eliminate the full pathname string embedded in the
executable, but I don't think the proposed FILE_MACRO_OFFSET will do
that either. Those get fixed by -fmacro-prefix-map=.
More information about the build-dev
mailing list