RFR: 8225429: Regression in disassembly quality

[Andrew Haley]

On 6/10/19 6:33 PM, Tom Rodriguez wrote:
>> Sure, I get that, but there are other issues. In particular, code comments
>> are disabled in product by macros such as
>>
>> #ifdef PRODUCT
>> #define BLOCK_COMMENT(str) /* nothing */
>> #define STOP(error) stop(error)
>> #else
>> #define BLOCK_COMMENT(str) block_comment(str)
>> #define STOP(error) block_comment(error); stop(error)
>> #endif
>>
>> I have no idea why these macros exist: unless debugging is enabled code
>> comments are gated by
>>
>> void CodeBuffer::block_comment(intptr_t offset, const char * comment) {
>>    if (_collect_comments) {
>>      _code_strings.add_comment(offset, comment);
>>
>> Maybe this is just historical baggage or it's a micro-optimization to
>> save space and a few machine cycles.
> 
> The block comment support definitely went through many iterations and 
> tweaks.  The ifdef's seem to be about binary size since the actual 
> recording support was already disabled in product.  Though that seems 
> like a pretty micro-optimization.

Today, yes, I think it probably is, and sustainable engineering would
win if we had another look.

> Block comment recording should probably be under a flag since it
> stores a bunch of extra strings associated with the code blobs which
> is completely unnecessary unless you are printing.  It would cool if
> it could be driven by PrintNMethods/PrintAssembly/PrintStubCode but
> having a separate flag might be more convenient.  Anyway, as you say
> there are some issues to be teased out in enabling block comments
> all the time.

Yes. I definitely intend to do some more in this area.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671