RFR(XL): 8181917: Refactor UL LogStreams to avoid using resource area

Marcus Larsson marcus.larsson at oracle.com
Wed Jun 21 10:18:50 UTC 2017


Hi,


On 2017-06-21 09:29, Thomas Stüfe wrote:
> Hi Marcus,
>
> thank you for reviewing!
>
> New webrev: 
> http://cr.openjdk.java.net/~stuefe/webrevs/8181917-refactor-ul-logstream/all.webrev.01/webrev/ 
> <http://cr.openjdk.java.net/%7Estuefe/webrevs/8181917-refactor-ul-logstream/all.webrev.01/webrev/>
>
> Delta to last version: 
> http://cr.openjdk.java.net/~stuefe/webrevs/8181917-refactor-ul-logstream/delta-all-00-to-01/webrev/ 
> <http://cr.openjdk.java.net/%7Estuefe/webrevs/8181917-refactor-ul-logstream/delta-all-00-to-01/webrev/>
>
> Changes:
>
> - classfile/loaderConstraints.cpp: as you suggested, I fixed up all 
> cases of "superfluous LogStream usage" I found and converted them to 
> direct LogTarget::print() calls.
>
> - classfile/sharedPathsMiscInfo.cpp: here I opted for removing any 
> notion of UL from this method, instead I just hand in an 
> outputStream*. Both the "is_enabled" check and the LogStream creation 
> is now handed in the caller frame. I also added a trailing cr().
>
> - gc/cms/compactibleFreeListSpace.cpp: removed the superfluous 
> ResourceMark
>
> - logging/logStream.hpp: enabled the private operator new() 
> declarations to disable heap allocations for class LogStream. I also 
> gave it a try, works fine, if you do new(), now you get a linker error.
>
> The rest of the changes is concerned with the removal of 
> "LogStreamCHeap" which is not needed anymore. Note that I found some 
> new instances of "unguarded printing" and I updated comments at 
> JDK-8182466 <https://bugs.openjdk.java.net/browse/JDK-8182466> .
>
> Thanks & Regards, Thomas

This looks great.

One last thing just hit me: We should have a unit test to cover the 
allocation case for LogStream. Preferably one that forces it to grow 
twice (once from stackbuffer, and once from heap), to make sure we 
exercise that code.

Thanks,
Marcus


More information about the hotspot-dev mailing list