RFR: 8221535: add steal tick related information to hs_error file [linux]

[David Holmes]

On 27/03/2019 8:04 pm, Baesken, Matthias wrote:
> Hi Thomas, interesting idea . David – would you be fine with  what 
> Thomas proposes ?

Isn't adding the missing fields to the existing os_perf code basically 
the same thing? I don't see any point in duplicating existing code like 
this.

David
-----

> Best regards, Matthias
> 
> *From:*Thomas Stüfe <thomas.stuefe at gmail.com>
> *Sent:* Mittwoch, 27. März 2019 10:28
> *To:* Baesken, Matthias <matthias.baesken at sap.com>
> *Cc:* hotspot-dev at openjdk.java.net
> *Subject:* Re: RFR: 8221535: add steal tick related information to 
> hs_error file [linux]
> 
> Hi Matthias,
> 
> this is a lot of coding for just one counter...
> 
> Also os_perf_linux.cpp does something very similar. However, that code 
> does not get the steal ticks you want, does not seem to care about 
> kernel differences and could also be improved.
> 
> Proposal: Why do you not tweak your new API to be a general purpose API 
> to read all ticks from /proc/stat. That would be way more useful. Later, 
> if we feel like it, could change os_perf_linux.cpp to use your function.
> 
> We did something similar for the statistical history in the SapMachine, see
> 
> https://github.com/SAP/SapMachine/blob/sapmachine/src/hotspot/os/linux/stathist_linux.cpp
> 
> . So I would suggest to have a function reading proc stat and returning 
> all values in a struct. That struct should have provisions for "value 
> not found" since which values you get depends in kernel version.
> 
> --------
> 
> Example:
> 
> struct value_t { bool valid; uint64_t v; }
> 
> struct cpu_values_t {
>   value_t user;
>   value_t nice;
>   value_t system;
>   value_t idle;
>   value_t iowait;
>   value_t steal;
>   value_t guest;
>   value_t guest_nice;
> };
> 
> void parse_proc_stat_cpu_line(const char* line, cpu_values_t* out) {
>    // Note: existence of some of these values depends on kernel version
>    out->user.valid = out->nice.valid = out->system.valid = 
> out->idle.valid = out->iowait.valid = out->steal.valid = 
> out->guest.valid = out->guest_nice.valid = false;
>    int user, nice, system, idle, iowait, irq, softirq, steal, guest, 
> guest_nice;
>    int num = ::sscanf(line, "cpu %d %d %d %d %d %d %d %d %d %d",
>                                &user, &nice, &system, &idle, &iowait, 
> &irq, &softirq, &steal, &guest, &guest_nice);
>    if (num >= 4) {
>      out->user.v = user; out->user.valid = true;
>      out->nice.v = nice; out->nice.valid = true;
>      out->system.v = system; out->system.valid = true;
>      out->idle.v = idle; out->idle.valid = true;
>      if (num >= 5) { // iowait (5) (since Linux 2.5.41)
>        out->iowait = iowait; out->iowait.valid = true;
>        if (num >= 8) { // steal (8) (since Linux 2.6.11)
>          out->steal = steal; out->steal.valid = true;
>          if (num >= 9) { // guest (9) (since Linux 2.6.24)
>            out->guest = guest; out->guest.valid = true;
>            if (num >= 10) { // guest (9) (since Linux 2.6.33)
>              out->guest_nice = guest_nice; out->guest_nice.valid = true;
>            }
>        }
>     }
> }
> 
> Cheers Thomas
> 
> On Wed, Mar 27, 2019 at 9:42 AM Baesken, Matthias 
> <matthias.baesken at sap.com <mailto:matthias.baesken at sap.com>> wrote:
> 
>     Hello, please review  the  following change .
> 
>     Recent Linux kernels provide in the /proc filesystem   so called
>     "steal" information, see the the proc man page :
> 
>        ..
>     "there is an eighth column, steal - stolen time, which is the time
>     spent in other operating systems when running in a virtualized
>     environment"
> 
>     The info was useful  in our  internal VM,  when looking into
>     performance issues  (especially in virtualized environments), so it
>     should be added to the hs_err file .
> 
>     Bug/webrev :
> 
>     https://bugs.openjdk.java.net/browse/JDK-8221535
> 
>     http://cr.openjdk.java.net/~mbaesken/webrevs/8221535.0/
> 
> 
>     Best Regards, Matthias
>