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

[coleen.phillimore at oracle.com]

On 4/9/19 9:55 AM, 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.
>
> 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.

I reviewed the original change and it's too bad we have to have full 
pathnames in the log messages, but this seems to be for the best.
Thanks,
Coleen

>
> /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
>>>>