Hi Matthias, I wonder if you should not do what David is suggesting and then put that whole code (the while loop) in a helper method. Below you have a calculation again using value2 (which I wonder what the added value of it is though) but anyway, that value2 could also be 0 at some point, no?
So would it not be best to just refactor the getAllStackTraces and calculate safepoint time in a helper method for both value / value2 variables? Thanks, Jc On Mon, Jul 29, 2019 at 7:50 PM David Holmes <[email protected]> wrote: > Hi Matthias, > > On 29/07/2019 8:20 pm, Baesken, Matthias wrote: > > Hello , please review this small test fix . > > > > The test > test/jdk/sun/management/HotspotRuntimeMBean/GetTotalSafepointTime.java > fails sometimes on fast Linux machines with this error message : > > > > java.lang.RuntimeException: Total safepoint time illegal value: 0 ms > (MIN = 1; MAX = 9223372036854775807) > > > > looks like the total safepoint time is too low currently on these > machines, it is < 1 ms. > > > > There might be several ways to handle this : > > > > * Change the test in a way that it might generate nigher safepoint > times > > * Allow safepoint time == 0 ms > > * Offer an additional interface that gives safepoint times with > finer granularity ( currently the HS has safepoint time values in ns , see > jdk/src/hotspot/share/runtime/safepoint.cpp SafepointTracing::end > > > > But it is converted on ms in this code > > > > 114jlong RuntimeService::safepoint_time_ms() { > > 115 return UsePerfData ? > > 116 Management::ticks_to_ms(_safepoint_time_ticks->get_value()) : -1; > > 117} > > > > 064jlong Management::ticks_to_ms(jlong ticks) { > > 2065 assert(os::elapsed_frequency() > 0, "Must be non-zero"); > > 2066 return (jlong)(((double)ticks / (double)os::elapsed_frequency()) > > 2067 * (double)1000.0); > > 2068} > > > > > > > > Currently I go for the first attempt (and try to generate higher > safepoint times in my patch) . > > Yes that's probably best. Coarse-grained timing on very fast machines > was bound to eventually lead to problems. > > But perhaps a more future-proof approach is to just add a do-while loop > around the stack dumps and only exit when we have a non-zero safepoint > time? > > Thanks, > David > ----- > > > Bug/webrev : > > > > https://bugs.openjdk.java.net/browse/JDK-8228658 > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8228658.0/ > > > > > > Thanks, Matthias > > > -- Thanks, Jc
