RFR (S): 8012260: ciReplay: Include PID into the name of replay data file

[Vladimir Ivanov]

I need one more "official" review. Any volunteers? :-)

Thanks!

Best regards,
Vladimir Ivanov

On 4/19/13 7:22 PM, Vladimir Kozlov wrote:
> Looks good.
>
> Thanks,
> Vladimir
>
> On 4/19/13 7:17 AM, Vladimir Ivanov wrote:
>> Updated fix:
>> http://cr.openjdk.java.net/~vlivanov/8012260/webrev.02/
>>
>> Did some additional cleanup: changed get_current_directory to accept
>> size_t instead of int as a buffer size.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> On 4/17/13 8:30 PM, Vladimir Kozlov wrote:
>>> On 4/17/13 9:03 AM, Vladimir Ivanov wrote:
>>>> I considered it redundant since fdopen returns NULL when a call fails
>>>> [1] and I already check replay_data_file. Was my reasoning wrong?
>>>
>>> My understanding was that you can't pass fd == -1 to fdopen() and I
>>> can't find any docs about such case so you may be right. And I am more
>>> concern about Windows/Cygwin implementation of fdopen.
>>>
>>> Vladimir
>>>
>>>>
>>>> I checked on MacOS and when (fd == -1) it produces the following error
>>>> message:
>>>> # Can't open file to dump replay data. Error: Bad file descriptor
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> [1] Excerpt from fdopen man page:
>>>> RETURN VALUES
>>>>       Upon successful completion fopen(), fdopen(), and freopen()
>>>> return
>>>> a FILE pointer.  Otherwise, NULL is returned and the global variable
>>>> errno is set to indicate the error.
>>>>
>>>> On 4/17/13 7:54 PM, Vladimir Kozlov wrote:
>>>>> Missing check (fd != -1) before fdopen:
>>>>>
>>>>> 1036       FILE* replay_data_file = fdopen(fd, "w");
>>>>>
>>>>> Vladimir
>>>>>
>>>>> On 4/17/13 8:19 AM, Vladimir Ivanov wrote:
>>>>>> Thanks for feedback, Vladimir!
>>>>>>
>>>>>> Here's updated version:
>>>>>> http://cr.openjdk.java.net/~vlivanov/8012260/webrev.01
>>>>>>
>>>>>> Best regards,
>>>>>> Vladimir Ivanov
>>>>>>
>>>>>> On 4/17/13 9:22 AM, Vladimir Kozlov wrote:
>>>>>>> Make it product flag since it is ON by default (and it is not
>>>>>>> diagnostic
>>>>>>> but error dump).
>>>>>>>
>>>>>>> Also why not reuse code which opens ErrorFile (hs_err*)?
>>>>>>>
>>>>>>> Compilation::compile_only_this_method() allocates fileStream on
>>>>>>> stack:
>>>>>>>
>>>>>>>    fileStream stream(fopen("c1_compile_only", "wt"));
>>>>>>>
>>>>>>> thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 4/16/13 5:01 PM, Vladimir Ivanov wrote:
>>>>>>>> http://cr.openjdk.java.net/~vlivanov/8012260/webrev.00
>>>>>>>> 43 lines changed: 16 ins; 21 del; 6 mod;
>>>>>>>>
>>>>>>>> Changed naming scheme for compilation replay data files from
>>>>>>>> 'replay.txt' to 'replay_pid%p.log', where %p is substituted w/
>>>>>>>> pid. It
>>>>>>>> allows to easily correspond replay file with hs_err file and makes
>>>>>>>> overwriting of existing files very unlikely.
>>>>>>>>
>>>>>>>> Also, converted ReplayDataFile & DumpReplayDataOnError flags from
>>>>>>>> develop to diagnostic, since they are useful in product binaries.
>>>>>>>> Did
>>>>>>>> some small cleanups.
>>>>>>>>
>>>>>>>> One question still bothers me. There's a memory allocation in
>>>>>>>> VMError::report_and_die() I don't know how to avoid:
>>>>>>>> +         fileStream* replay_data_stream = new
>>>>>>>> (ResourceObj::C_HEAP,
>>>>>>>> mtCompiler) fileStream(replay_data_file);
>>>>>>>>
>>>>>>>> It's not new, I just moved it up from auxiliary method. Any ideas
>>>>>>>> how to
>>>>>>>> make VMError::report_and_die() allocation-free again? :-)
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Vladimir Ivanov
>>>>>>>>