RFR: 8138916: Logging write function does not allow for long enough messages

[Mikael Gerdin]

Dmitry,


On October 28, 2015 4:57:33 PM Dmitry Samersoff 
<dmitry.samersoff at oracle.com> wrote:

> Mikael,
>
>> If your libc only provides a ::malloc which takes a
>> lock on every allocation then you should switch OS (or use libumem on
>> Solaris).
>
> Thank you for advice.
>
> Default glibc malloc (ptmalloc) maintain multiple arenas. Each arena has
> it's own lock and it allows multiple threads do allocation at the same
> time, but in heavy multi-threaded environment like hotspot number of
> arenas is less than number of threads (see MALLOC_ARENA_MAX) so
> extensive use of malloc still could be an issue.

Thanks for the detailed description!
/Mikael

>
> -Dmitry
>
>
> On 2015-10-27 15:57, Mikael Gerdin wrote:
>> On 2015-10-27 12:52, Dmitry Samersoff wrote:
>>> Rachel,
>>>
>>> In worst case (windows, overflow) the code would parse format string
>>> four times. Also NEW_C_HEAP_ARRAY takes a lock and might become a
>>> performance bottleneck.
>>
>> NEW_C_HEAP_ARRAY does not take any VM lock but is dependent on the libc
>> ::malloc function. If your libc only provides a ::malloc which takes a
>> lock on every allocation then you should switch OS (or use libumem on
>> Solaris).
>> /Mikael
>>
>>
>>>
>>> POSIX equivalent of _vscprintf(fmt, args) is vsnprintf(NULL, 0, fmt,
>>> args)
>>>
>>> So it might be better to create os::log_vcprintf(fmt, args), then write
>>> code at log.hpp as:
>>>
>>> int buf_len = os::log_vcprintf(fmt, args);
>>> assert(buf_len < 2*page_size, "Stack overflow in logging");
>>> char buf[buf_len];
>>> vsprintf(buf, fmt, args);
>>>
>>> -Dmitry
>>>
>>>
>>>
>>> On 2015-10-26 23:58, Rachel Protacio wrote:
>>>> Hello, all,
>>>>
>>>> Please review this fix to the faulty write function from the Unified
>>>> Logging framework.
>>>>
>>>> Summary: In logging/log.hpp, the logging vwrite function previously
>>>> asserted that the buffer remains within 512 characters, which is too
>>>> short for logging message of non-pre-determined length, e.g. for vtable
>>>> and itable function names. Now, the function resizes the buffer to the
>>>> appropriate length, asserting only that the resulting buffer is valid.
>>>> Includes tag "logtest" to test that logging can print an output of 518
>>>> characters.
>>>>
>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8138916/
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8138916
>>>>
>>>> Includes own acceptance test, and passes JPRT and remote hotspot/test
>>>> tests.
>>>>
>>>> Thank you,
>>>> Rachel
>>>
>>>
>>
>
>
> --
> Dmitry Samersoff
> Oracle Java development team, Saint Petersburg, Russia
> * I would love to change the world, but they won't give me the sources.