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