On Tue, Mar 18, 2014 at 11:37 AM, Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> wrote: > On 2014-03-14 17:55, Volker Simonis wrote: >> >> Hi Magnus, >> >> thanks for your detailed comments. >> >> I generally agree with your analysis of the problem. >> >> However, as you correctly noticed, the 'optimized' configuration is >> actually a "HotSpot only" concept. It is used to enable extra >> options/functionality which should not be shipped to the customer. >> >> Everything flagged with "#ifndef PRODUCT" in HotSpot is enabled by >> default in the "slowdebug" and "fastdebug" configuration - so no >> problem. But the Hotspot has this extra configuration called >> 'optimized' which is exactly like a release build, but with the extra, >> "#ifndef PRODUCT" stuff enabled. This may be useful for example for >> testing/benchmarking new functionality (i.e. you don't want to >> benchmark with a debug build). >> >> There's no general 'optimized' concept for the jdk and the other >> repositories as far as I can see. The "PRODUCT" macro is only used in >> src/share/native/com/sun/java/util/jar/pack/* within the 'jdk/' >> repository (and it is used there with a DEBUG/ASSERT semantics). >> >> I'd therefore propose to slightly modify my change as follows: >> >> optimized ) >> DEBUG_LEVEL="release" >> VARIANT="OPT" >> FASTDEBUG="false" >> DEBUG_CLASSFILES="false" >> BUILD_VARIANT_RELEASE="-optimized" >> HOTSPOT_DEBUG_LEVEL="optimized" >> HOTSPOT_EXPORT="optimized" >> ;; > > > Okay. This seems more robust. > > However, I think it's a bit scary to change the value of DEBUG_LEVEL, it's > sort of considered to be a straight mapping from the configure command line > option. I'm okay with it, but it needs to be done more explicitely. Either a > comment at the place where you modify it to "release", or -- even better -- > a separate if statement after the case block that clearly shows that > optimized is replaced with release, with proper comments. >
Done. Please see the new webrev at: http://cr.openjdk.java.net/~simonis/webrevs/8037298_3/ > Also, you do not need to do AC_SUBST unless you publish the value in > spec.gmk. If you want to use the variable HOTSPOT_DEBUG_LEVEL in the > makefiles, you need to add it to spec.gmk.in (or hotspot-spec.gmk.in). If > not, you do not need the AC_SUBST. > You're right. I only need HOTSPOT_DEBUG_LEVEL in help.m4 and for that purpose AC_SUBST isn't neccessary, so I removed it. > Apart from that, the change looks good. > Thanks. Could you please push this change for me because it requires the regeneration of generated-configure.sh and potentially also the regeneration of closed configure files which I can't do. Regards, Volker > /Magnus
