RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[Yasumasa Suenaga]

2019年4月10日(水) 13:55 David Holmes <david.holmes at oracle.com>:
>
> Hi Erik,
>
> On 9/04/2019 11:55 pm, Erik Joelsson wrote:
> > Hello,
> >
> > Here is a new webrev with an even simpler change:
> >
> > http://cr.openjdk.java.net/~erikj/8221851/webrev.06/
> >
> > I decided to just remove the usage of THIS_FILE from exceptions.hpp
> > since it did not work well. It currently does not work at all on Windows
> > or Macosx builds. On Linux it invalidates the precompiled header, which
> > is the main issue I'm trying to fix.
> >
> > As for the truncation issue, with the patch above, truncation will again
> > happen on Solaris and Linux if compiled with GCC <8. However, as Thomas
> > pointed out, there is already a review out for fixing the truncation
> > problems for real.
>
> As JDK-8220762 is soon to be fixed I'm okay with this temporarily
> regressing JDK-8204551. I've cc'd Yasumasa so he knows.

I think webrev.06 is reasonable to me.
In addition, 8220093 has been fixed in JDK 13, so OpenJDK binary for
Linux (rovided by jdk.java.net at least) will be compiled with GCC 8.
I guess the regression in 8221851 is minimal, and we can ignore it
after 8220762.
(I've reviewed 8220762 in hotspot-runtime-dev mailing list.)


Thanks,

Yasumasa


> Thanks,
> David
> -----
>
> >
> > The reason I tried to pursue finding a solution with shortening of the
> > __FILE__ path was that I thought it would be beneficial in more
> > locations, but it now seems like more trouble than it's worth and not
> > actually a feature anyone seem to want. I do not want to spend more time
> > one this, I just want the precompiled header to start working again.
> > Yes, this regresses the truncation issue somewhat, but I do not think a
> > broken solution should be left in there.
> >
> > /Erik
> >
> > On 2019-04-08 15:27, David Holmes wrote:
> >> Hi Erik,
> >>
> >> On 9/04/2019 8:08 am, Erik Joelsson wrote:
> >>> New webrev with "_simple_basename":
> >>> http://cr.openjdk.java.net/~erikj/8221851/webrev.05/
> >>
> >> Given the usage is typically of the form:
> >>
> >>  Exceptions::_throw(THREAD_AND_LOCATION, e);
> >>
> >> which will expand to:
> >>
> >>  Exceptions::_throw(THREAD, _simple_basename(__FILE__), __LINE__, e);
> >>
> >> what does the compiler actually generate for this at the call sites?
> >> I'm struggling with the addition of an inline function to a .hpp which
> >> we generally frown upon and have been working to remove.
> >>
> >> What affect does this have on code size?
> >>
> >> Thanks,
> >> David
> >> -----
> >>
> >>> /Erik
> >>>
> >>> On 2019-04-08 12:20, Erik Joelsson wrote:
> >>>> On 2019-04-08 11:40, Kim Barrett wrote:
> >>>>>> On Apr 8, 2019, at 10:28 AM, Erik Joelsson
> >>>>>> <erik.joelsson at oracle.com> wrote:
> >>>>>>
> >>>>>> Hello,
> >>>>>>
> >>>>>> On 2019-04-05 15:46, Kim Barrett wrote:
> >>>>>>> Assuming all that, consider instead putting this_file_helper in
> >>>>>>> exceptions.hpp (perhaps with a better name?), don't bother with
> >>>>>>> THIS_FILE, and define THREAD_AND_LOCATION as
> >>>>>>>
> >>>>>>> #define THREAD_AND_LOCATION THREAD, this_file_helper(__FILE__),
> >>>>>>> __LINE__
> >>>>>>>
> >>>>>> Moved to exceptions.hpp, renamed to "basename", and removed the
> >>>>>> THIS_FILE macro.
> >>>>> “basename” might not count as a “better name”, as it conflicts with
> >>>>> a POSIX function,
> >>>>> even though we don’t presently seem to be using that anywhere that
> >>>>> I could find.
> >>>>>
> >>>>>
> >>>> How about "simple_basename" then? Or just prefix with an underscore?
> >>>> The idea is to keep it internal to the headerfile, but I'm not
> >>>> familiar with conventions in Hotspot to know how you usually
> >>>> prefix/namespace things as private.
> >>>>
> >>>> /Erik
> >>>>