RFR 8026324: hs_err improvement: Add summary section to hs_err file
David Holmes
david.holmes at oracle.com
Fri Jul 17 07:14:58 UTC 2015
Hi Coleen,
Looks good.
A few minor comments ...
On 17/07/2015 5:20 AM, Coleen Phillimore wrote:
> 8026333: hs_err improvement: Print GC Strategy
> 8026336: hs_err improvement: Print compilation mode, server, client or
> tiered
> Summary: Added command line, summary cpu and os information to summary
> section. Moved time of crash and duration in summary section. Add GC
> strategy and compiler setting (tiered) to enhanced version string in
> error report. Moved the stack trace sooner in hs_err file.
>
> See new summary format in bug report and some samples.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8026324.01/
> bug link https://bugs.openjdk.java.net/browse/JDK-8026324
>
> Tested with RBT quick tests (tests that run on all platforms). Verified
> on crash logs on all platforms available inside Oracle either with
> crashing test or C++ program with same new code (or both), and various
> scenarios of missing cpuinfo/distribution files.
src/os/aix/vm/os_aix.cpp
+ // This looks good
+ void os::get_summary_cpu_info(char* buf, size_t buflen) {
+ // This looks good
It doesn't look _that_ good :)
---
src/os/bsd/vm/os_bsd.cpp
1709 size_t size = sizeof(mhz);
1710 int mib[] = { CTL_HW, HW_CPU_FREQ };
1711 size = sizeof(mhz);
Line #1711 is redundant
1713 mhz = 0;
Wouldn't a default of 1 be safer incase anyone parses this and does
arithmetic?
BTW took me a while to realize these "if" bodies were setting defaults
for when sysctl failed.
1721 strncpy(model, cpu_arch, sizeof(model));
where did cpu_arch come from??
---
src/os/linux/vm/os_linux.cpp
2066 if (file == NULL) break;
2068 if (_print_ascii_file(file, st)) return;
Can you put the break/return on a new line please.
More generally for some reason today I have an aversion to "if (x)
foo();" being on a single line :) But control flow in particular I
prefer to see on its own line.
2082 char buf[257];
Where does the 257 line length come from? Is this a defined limit for
the release file?
2218 char buf[257];
Where does the 257 line length come from? The flags line on my linux box
is 410 characters.
2247 ARM32_ONLY("arm32") PPC_ONLY("PPC64") AARCH64_ONLY("AARCH64"),
length);
May I suggest "ARM" for ARM32 and it should be "AArch64".
---
src/os/posix/vm/os_posix.cpp
241 // could use uname as in above also
242 return (gethostname(buf, buflen) != -1);
I would suggest using uname given that Linux Glibc implements
gethostname using uname. gethostname also has unclear semantics
regarding nul-termination if the name is truncated.
---
src/os/solaris/vm/os_solaris.cpp
1978 char tmp[256];
1979 if (fgets(tmp, 256, fp)) {
256 this time? :) And the second 256 should sizeof(tmp).
Can you add a comment that you are getting only the first line, please.
2028 #ifdef AMD64
2029 snprintf(buf, buflen, "x86 64 bit %d MHz", clock);
2030 #elif (defined __sparc) && (defined _LP64)
2031 snprintf(buf, buflen, "Sparcv9 64 bit %d MHz", clock);
2032 #else
2033 #error "Invalid solaris configuration"
I think "ifdef AMD64 else // must be sparc" will do here.
Aside: it is truly horrible how little useful processor info you can get
from Solaris. It still refers to i386 and i387!
---
src/os/windows/vm/os_windows.cpp
1620 if (GetComputerNameEx(ComputerNameDnsHostname, buffer, &size)) {
Is it worth turning the above into a call to os::get_host_name to avoid
the duplicate call?
---
src/share/vm/runtime/arguments.cpp
I'm not quite sure what gets printed for print_summary_on, or in what
format. Can you given an example please showing where each piece comes
from (jvm_flags vs jvm_args vs java_command).
---
src/share/vm/utilities/vmError.cpp
691 STEP(250, "(printing register info)")
699 STEP(260, "(printing registers, top of stack, instructions near pc)")
What's the difference between registers and register info ??
564 STEP(140, "(printing VM options)" )
816 STEP(380, "(printing VM options)" )
One of those should be changed.
Thanks,
David
-----
> Thanks,
> Coleen
More information about the hotspot-dev
mailing list