RFR(trivial): 8231024: Improve the debug info when the output is truncated

[Jie Fu]

Hi Ioi,

Thank you so much.

I would be appreciated if you could sponsor it.

Thanks a lot.
Best regards,
Jie

On 2019/9/17 下午2:21, Ioi Lam wrote:
> Hi Jie,
>
> Looks good to me, too. Thanks
>
> - Ioi
>
> On 9/16/19 10:40 PM, David Holmes wrote:
>> Hi Jie,
>>
>> That seems fine to me.
>>
>> Thanks,
>> David
>> -----
>>
>> On 17/09/2019 11:58 am, Jie Fu wrote:
>>> Hi Ioi and David,
>>>
>>> Thank you very much for your review and valuable comments.
>>>
>>> Updated: http://cr.openjdk.java.net/~jiefu/8231024/webrev.02/
>>>   -1) Change the error message which is somewhat misleading
>>>   -2) Rename "written" to "required_len"
>>>
>>> - Before the patch
>>> ------------------------------------
>>> [2019-09-14 16:07:55,752] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
>>> warning: increase O_BUFLEN in ostream.hpp -- output truncated
>>> [2019-09-14 16:07:55,758] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
>>> warning: increase O_BUFLEN in ostream.hpp -- output truncated
>>> [2019-09-14 16:07:55,763] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
>>> warning: increase O_BUFLEN in ostream.hpp -- output truncated
>>> ...
>>> ------------------------------------
>>>
>>> - After the patch
>>> ------------------------------------
>>> [2019-09-17 09:49:21,567] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
>>> warning: outputStream::do_vsnprintf output truncated -- buffer 
>>> length is 2000 bytes but 2013 bytes are needed.
>>> [2019-09-17 09:49:21,571] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
>>> warning: outputStream::do_vsnprintf output truncated -- buffer 
>>> length is 2000 bytes but 2031 bytes are needed.
>>> [2019-09-17 09:49:21,576] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
>>> warning: outputStream::do_vsnprintf output truncated -- buffer 
>>> length is 2000 bytes but 2049 bytes are needed.
>>> ...
>>> ------------------------------------
>>>
>>> Could you please review it and give me some advice?
>>>
>>> Thanks a lot.
>>> Best regards,
>>> Jie
>>>
>>> On 2019/9/17 上午7:20, Ioi Lam wrote:
>>>>
>>>>
>>>> On 9/16/19 12:44 AM, David Holmes wrote:
>>>>> On 16/09/2019 5:09 pm, Jie Fu wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> Sorry for the confusion.
>>>>>
>>>>> The confusion is mine, not from you :)
>>>>>
>>>>>> On 2019/9/16 下午2:40, David Holmes wrote:
>>>>>>> Okay I'm confused. I had assumed that the passed in buffer size 
>>>>>>> must be different in each case because of the 2014/2032/2050 
>>>>>>> values in the message, but in fact they are all sized at 2000. 
>>>>>> According to the source code, the buffer size can be either 
>>>>>> 2000(O_BUFLEN) [1] or 2048(BUFLEN) [2].
>>>>>> So it isn't always 2000.
>>>>>
>>>>> It is 2000 is all three examples of the output that you gave.
>>>>
>>>> xmlStream::va_tag() uses 2*K.
>>>>
>>>>>
>>>>>>> That makes no sense
>>>>>> I think it makes sense.
>>>>>> It is still unclear which one (O_BUFLEN or BUFLEN?) should be 
>>>>>> increased if the current buffer size wasn't printed.
>>>>>
>>>>> You need to know the original caller that passed in the buffer size.
>>>>>
>>>>> But we can't just keep increasing the buffer size. We probably 
>>>>> need to examine the callers to see what amount of information is 
>>>>> trying to be passed through. Then decide whether to adjust what 
>>>>> the callers are passing through, or adjust the buffer size - 
>>>>> perhaps on a callsite by callsite basis rather than just bumping 
>>>>> O_BUFLEN or BUFLEN.
>>>>>
>>>>
>>>> I think the message should stop saying "increase O_BUFLEN" -- 
>>>> probably the reviewers will reject all future increase of O_BUFLEN 
>>>> and will suggest fixing the caller of outputStream::do_vsnprintf 
>>>> instead.
>>>>
>>>> Callers of outputStream::do_vsnprintf don't necessarily use 
>>>> O_BUFLEN. E.g., xmlStream::va_tag allocates its own buffer. 
>>>> VMError::report_and_die() also uses its own buffer -- it uses 
>>>> O_BUFLEN today, but there's no guarantee or requirement that it 
>>>> always does that.
>>>>
>>>> How about saying something like "outputStream::do_vsnprintf output 
>>>> truncated -- buffer length is 2000 bytes but 2100 bytes are needed."
>>>>
>>>>
>>>>>>> - how can we have written e.g. 2014 bytes to a buffer of only 
>>>>>>> 2000 bytes? 
>>>>>>
>>>>>> The output is truncated since the current buffer size is too 
>>>>>> small (e.g., 2000 < 2014).
>>>>>
>>>>> Ah sorry - I'd missed that aspect of vsnprintf. We're calling:
>>>>>
>>>>>  102     int written = os::vsnprintf(buffer, buflen, format, ap);
>>>>>
>>>>> but it isn't "written" when it exceeds buflen, it's the required 
>>>>> buffer size.
>>>>>
>>>>
>>>> I think the variable "written" should be renamed to "required".
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>>
>>>>>>> Do we have a bug in os::vsnprintf?
>>>>>>
>>>>>> I don't think so.
>>>>>>
>>>>>> Thanks a lot.
>>>>>> Best regards,
>>>>>> Jie
>>>>>>
>>>>>> [1] 
>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/24df796eef3d/src/hotspot/share/utilities/ostream.cpp#l124 
>>>>>>
>>>>>> [2] 
>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/24df796eef3d/src/hotspot/share/utilities/xmlstream.cpp#l138 
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>
>