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

[Ioi Lam]

Sure I’ll test and push that tomorrow.

Thanks
Ioi

> On Sep 16, 2019, at 11:28 PM, Jie Fu <fujie at loongson.cn> wrote:
> 
> 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 
>>>>>>> 
>>>>>>> 
>>>>> 
>>>> 
>> 
>