[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(M): 8182651: Add TRACE_ONLY conditional macro to support more fine-grained INCLUDE_TRACE pro
From:       Claes Redestad <claes.redestad () oracle ! com>
Date:       2017-06-22 14:25:24
Message-ID: 610a5ba7-5c9c-7d67-d74d-71e0dfe19a48 () oracle ! com
[Download RAW message or body]

Hi Markus,

On 2017-06-22 14:44, Markus Gronlund wrote:
> Hi again,
> 
> Here is an updated webrev02, mainly with additional includes of \
> "utilities/macros.hpp". 
> Webrev02: http://cr.openjdk.java.net/~mgronlun/8182651/webrev02/

FWIW, this looks good to me!

/Claes

> 
> Also, there are small updates compared to webrev01 for the following files:
> 
> SystemDictionary.cpp:
> 
> Removed inadvertently added "//" comment (thanks Claes)
> 
> http://cr.openjdk.java.net/~mgronlun/8182651/webrev02/src/share/vm/classfile/systemDictionary.cpp.frames.html
>  
> Unsafe.cpp:
> 
> Restored conditional evaluation on parker object (thanks again Claes)
> 
> http://cr.openjdk.java.net/~mgronlun/8182651/webrev02/src/share/vm/prims/unsafe.cpp.frames.html
>  
> 
> traceEvent.hpp:
> 
> Misc removal
> 
> http://cr.openjdk.java.net/~mgronlun/8182651/webrev02/src/share/vm/trace/traceEvent.hpp.frames.html
>  
> Thanks again
> Markus
> 
> -----Original Message-----
> From: Markus Gronlund
> Sent: den 22 juni 2017 10:06
> To: Stefan Karlsson
> Cc: hotspot-runtime-dev@openjdk.java.net
> Subject: RE: RFR(M): 8182651: Add TRACE_ONLY conditional macro to support more \
> fine-grained INCLUDE_TRACE programming 
> Thanks Stefan,
> 
> I'll do a pass to ensure all files include macros.hpp directly where necessary.
> 
> Cheers
> Markus
> 
> -----Original Message-----
> From: Stefan Karlsson
> Sent: den 22 juni 2017 09:00
> To: Markus Gronlund; hotspot-runtime-dev@openjdk.java.net
> Subject: Re: RFR(M): 8182651: Add TRACE_ONLY conditional macro to support more \
> fine-grained INCLUDE_TRACE programming 
> Hi Markus,
> 
> I opened the first file in this patch and saw that it didn't explicitly include \
> macros.hpp. That is risky, and all checks for the INCLUDE_ defines need to be \
> preceded with an include to macros.hpp. 
> Otherwise, you run the risk of not compiling code inside blocks like:
> #if INCLUDE_TRACE
> // do stuff
> #endif
> 
> even though compilation of trace code is enabled.
> 
> Thanks,
> StefanK
> 
> On 2017-06-21 20:44, Markus Gronlund wrote:
> > Greetings,
> > 
> > 
> > 
> > This is a follow-up suggestion to an earlier discussion [1] regarding the \
> > conditional vs. unconditional use of trace-related code in general. 
> > 
> > 
> > Webrev: http://cr.openjdk.java.net/~mgronlun/8182651/webrev01/
> > 
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8182651
> > 
> > 
> > 
> > This will introduce the TRACE_ONLY macro to better handle conditional compilation \
> > as well as giving some more flexibility to the ways of working with events in the \
> > VM. 
> > 
> > 
> > Thanks
> > 
> > Markus
> > 
> > 
> > 
> > [1]
> > 
> > http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2017-May/02
> > 3328.html
> > 
> > 
> > 


[prev in list] [next in list] [prev in thread] [next in thread]