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

[Ioi Lam]

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