RFR(L): 8198691: CodeHeap State Analytics

[Tobias Hartmann]

I've quickly ran this through our testing and there are some build failures:

jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(1710) : warning C4101: 'frameLine' :
unreferenced local variable
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(1711) : warning C4101: 'textLine' :
unreferenced local variable
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(1922) : warning C4101: 'frameLine' :
unreferenced local variable
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(1923) : warning C4101: 'textLine' :
unreferenced local variable
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(2094) : warning C4101: 'frameLine' :
unreferenced local variable
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(2095) : warning C4101: 'textLine' :
unreferenced local variable
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(2282) : warning C4101: 'frameLine' :
unreferenced local variable
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(2283) : warning C4101: 'textLine' :
unreferenced local variable
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(2479) : warning C4101: 'frameLine' :
unreferenced local variable
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(2480) : warning C4101: 'textLine' :
unreferenced local variable
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(2699) : warning C4267: '+=' : conversion
from 'size_t' to 'int', possible loss of data
jib > t:/workspace/open/src/hotspot/share/memory/heap.cpp(2700) : warning C4267: '+=' : conversion
from 'size_t' to 'int', possible loss of data

Best regards,
Tobias

On 06.03.2018 11:25, Tobias Hartmann wrote:
> Hi Lutz,
> 
> first of all, this is a very nice enhancement!
> 
> I haven't looked at the code in detail yet (will wait for the next webrev). Here are some comments:
> - The documentation is great and should at least go into the release notes (I've added the
> release-not=yes label). I think we should also add something to the Tools Reference Guide [1] or
> even the JVM Guide [2].
> - As Robbin already pointed out, please use unified logging instead of PrintCodeHeapState. I think
> that would also allow to specify <function> and <granularity> on the command line.
> - I think it would be nice to print some/all of this new information if the code cache is full at
> CodeCache::report_codemem_full() if PrintCodeHeapState is enabled. We had several customer reported
> issues in the past where the compilers were disabled although there still was some free space in the
> code cache. We were never able to reproduce/analyze but always expected this to be due to high
> fragmentation.
> - JFR integration would be nice but that should be done in a separate RFE
> - It's a style question but I would prefer the pointer asterisk at the type not the argument. For
> example, "outputStream* out" instead of "outputStream *out"
> - heap.hpp: please put the commas after the definitions in the enum
> 
> Thanks,
> Tobias
> 
> [1] https://docs.oracle.com/javase/9/tools/java.htm#JSWOR624
> [2]
> https://docs.oracle.com/javase/9/vm/java-virtual-machine-technology-overview.htm#JSJVM-GUID-982B244A-9B01-479A-8651-CB6475019281
> 
> 
> On 01.03.2018 18:17, Schmidt, Lutz wrote:
>> Dear all,
>>
>>  
>>
>> may I please request reviews for this quite voluminous enhancement:
>>
>>  
>>
>> Bug:     https://bugs.openjdk.java.net/browse/JDK-8198691
>>
>> Webrev:  http://cr.openjdk.java.net/~lucy/webrevs/8198691.00/index.html  
>>
>>  
>>
>> Don’t get afraid! Most of the logic is new and isolated in a big, separate block in heap.cpp. The
>> changes to other files are not difficult to understand.
>>
>>  
>>
>> If you need information about what this enhancement does and how it can be used, please refer to the
>> bug description. There I have attached some documentation which will greatly help with understanding
>> the code.
>>
>>  
>>
>> Thank you!
>>
>> Lutz
>>
>>  
>>
>>  
>>
>>  
>>