RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC
Kim Barrett
kim.barrett at oracle.com
Wed Apr 3 23:34:14 UTC 2019
> On Apr 3, 2019, at 9:51 AM, Erik Joelsson <erik.joelsson at oracle.com> wrote:
> On 2019-04-02 16:02, Kim Barrett wrote:
>>>
>>> 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. […]
>
> This kind of solution will also work. It's worth noting that your fix will work like basename and just print the filename (as the current state in hotspot does for exceptions if compiled with GCC). I think printing the complete relative path to the workspace root is an improvement over this, and I think the only reason it's currently printing just the filename is that that's what appeared available as an alternative. The issue with truncated paths is mostly happening on our internal Mach5 builds where the workspace root is usually located in an extremely deep path hierarchy.
I disagree that it's mostly a problem on our internal Mach5 builds.
There are paths in my development repos that exceed the clipped size
in the examples in that bug, and I don't think I have a very deep
hierarchy. Using a workspace-relative path in exception messages might
be enough to still address JDK-8204551 though.
> Additionally, I would like to replace all (or at least most) instances of __FILE__ with my new THIS_FILE, and I suspect some other places would be more performance sensitive. Do you see any problem with doing that substitution?
Given that our build system is mostly unhappy with basename collisions
(other than open/custom overrides), I don't think there's much of a
usage problem with using a basename-like approach everywhere either.
While researching what various platforms do in this area, I ran across
a suggestion that debug builds should maybe use full paths and release
builds use shortened paths. That seems like an interesting idea, though
we still want short paths in the exceptions.hpp case.
I tend to expect (modern) compilers to be pretty good at obvious
optimizations, though admit I'm occasionally sadly disappointed.
>> 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=.
>
> Correct. While solving this universally would be nice, it wasn't the goal of this bug. The main goal was to get precompiled headers to work with GCC again.
I agree that either approach does that. I looked at the webrev and
it's a bunch of build system changes that I don't feel qualified to
review, so I suggested an alternative that I understand. We each have
our preferred tools. I don't object to the build-system-based
approach, just can't give an actual thumbs-up.
More information about the build-dev
mailing list