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

[Vladimir Kozlov]

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