[OSHI] Add bytesRead and bytesWritten to OSProcess #262

plamenko · 2016-10-07T10:27:11Z

Summary:
This change adds bytesRead and bytesWritten fields to OSProcess.
An implementation is provided for MacProcess which uses proc_pid_rusage(pid, RUSAGE_INFO_V2, ...).
Note that RUSAGE_INFO_V2 is only available on MacOSX 10.9+.

Test Plan:
Successfully obtained process io stats on MacOSX 10.10 and 10.11.
Verified that mvn test succeeds.

codecov-io · 2016-10-07T10:31:06Z

Current coverage is 84.46% (diff: 100%)

Merging #262 into master will increase coverage by 0.03%

@@             master       #262   diff @@
==========================================
  Files            23         23          
  Lines          1047       1056     +9   
  Methods           0          0          
  Messages          0          0          
  Branches        189        191     +2   
==========================================
+ Hits            884        892     +8   
  Misses           72         72          
- Partials         91         92     +1

Powered by Codecov. Last update 8b28442...345e997

coveralls · 2016-10-07T10:31:33Z

Coverage remained the same at 91.977% when pulling 76c9b5a on plamenko:proc_io into 8b28442 on oshi:master.

dbwiddis

Looks good, with the comments indicated. Are you able to do the other OS's or just Mac?

dbwiddis · 2016-10-07T17:06:03Z

oshi-core/src/main/java/oshi/software/os/mac/MacOperatingSystem.java

@@ -107,12 +108,18 @@ public OSProcess getProcess(int pid) {
                }
            }
        }
+        long bytesRead = 0, bytesWritten = 0;
+        RUsageInfoV2 rUsageInfoV2 = new RUsageInfoV2();
+        if (0 == SystemB.INSTANCE.proc_pid_rusage(pid, SystemB.RUSAGE_INFO_V2, rUsageInfoV2)) {


Should we check for OS version >= 10.9 here? What happens if this is run on an earlier version? I suspect an exception (if the method doesn't exist) or possibly it populates a smaller structure and would give memory access errors/segfault on the next two lines.

We could use a utility method to find OS X version (need to check for 12+ for #260) so might as well dual-purpose that.

I added a version check.

dbwiddis · 2016-10-07T17:11:35Z

oshi-core/src/main/java/oshi/jna/platform/mac/SystemB.java

+     *            holds results
+     * @returns 0 on success; or -1 on failure, with errno set to indicate the specific error.
+     */
+    int proc_pid_rusage(int pid, int flavor, Structure buffer);


Should use type RUsageInfoV2 here. Or, if preferred, RUsageInfoT (define to extend Structure) which the V2 class can subsequently extend.

dbwiddis · 2016-10-07T17:11:54Z

oshi-core/src/main/java/oshi/jna/platform/mac/SystemB.java

@@ -127,6 +130,37 @@
        }
    }

+    class RUsageInfoV2 extends Structure {
+        public byte ri_uuid[] = new byte[16];


Should be public byte[] ri_uuid = ...

dbwiddis · 2016-10-11T15:22:05Z

Forgot to note in the code review ... you'll also need to modify the oshi-json code (and config file) to output the additional fields.

Summary: This change adds bytesRead and bytesWritten fields to OSProcess. An implementation is provided for MacProcess which uses `proc_pid_rusage(pid, RUSAGE_INFO_V2, ...)`. Note that RUSAGE_INFO_V2 is only available on MacOSX 10.9+. Test Plan: Successfully obtained process io stats on MacOSX 10.10 and 10.11. Verified that `mvn test` succeeds.

plamenko · 2016-10-12T16:31:44Z

Yeah, I'll try to make a pull request for Linux and possibly windows.

plamenko · 2016-10-12T16:44:38Z

I amended the commit and you can see the new changes, but for some reason GitHub didn't comment here that the pull request got updated and it still shows plamenko committed 23 days ago instead of showing the time of the last update.

coveralls · 2016-10-12T16:58:46Z

Coverage remained the same at 91.977% when pulling c5f883b on plamenko:proc_io into 8b28442 on oshi:master.

Test Plan: Test app that consumes io/cpu/mem resources: mem size: 401 bytes read: 42991616 bytes written: 42991616 ^C real 0m41.241s user 0m2.007s sys 0m10.478s OSHI: pid: 1619028, MEM Resident: 411 MB, MEM Size: 829 MB, CPU Real: 41.299 s, CPU User: 2.0 s, CPU Sys: 10.43 s, CPU Total: 12.43 s, IO Read: 42 MB, IO Write: 41 MB, IO Total: 83 MB,

plamenko · 2016-10-12T20:32:55Z

Okay I added proc io stats for Linux too and verified it works:

My test app that consumes io/cpu/mem resources:

mem size: 401
bytes read: 42991616
bytes written: 42991616
^C
real  0m41.241s
user  0m2.007s
sys 0m10.478s

OSHI with these two commits:

pid: 1619028, MEM Resident: 411 MB, MEM Size: 829 MB, CPU Real: 41.299 s, CPU User: 2.0 s, CPU Sys: 10.43 s, CPU Total: 12.43 s, IO Read: 42 MB, IO Write: 41 MB, IO Total: 83 MB

I also found out a bug in reporting RSS on Linux as the number OSHI reported were suspicious. Turns out stats gives RSS in pages, not in bytes, so I fixed that too:

http://man7.org/linux/man-pages/man5/proc.5.html

(23) vsize  %lu
          Virtual memory size in bytes.

(24) rss  %ld
          Resident Set Size: number of pages the process has
          in real memory.  This is just the pages which count
          toward text, data, or stack space.  This does not
          include pages which have not been demand-loaded in,
          or which are swapped out.

coveralls · 2016-10-12T21:18:02Z

Coverage increased (+0.07%) to 92.045% when pulling 345e997 on plamenko:proc_io into 8b28442 on oshi:master.

Summary: oshi/oshi#262 was merged so now we can have process disk i/o stats as well. Test Plan: checked buck-0.log: [2016-10-13 17:01:23.702][vrbos][command:839d219a-8cf7-4a6e-ad49-a0fd3e4baf89][tid:25][com.facebook.buck.util.perf.ProcessTracker] Process resource consumption: [/opt/homebrew/bin/python2, ...] ProcessResourceConsumption{memResident=29298688, memSize=2584113152, cpuReal=9270, cpuUser=3255, cpuSys=1461, cpuTotal=4716, ioBytesRead=46559232, ioBytesWritten=110592, ioTotal=46669824} Reviewed By: Coneko fbshipit-source-id: 6b9f7dc

dbwiddis requested changes Oct 7, 2016

View reviewed changes

dbwiddis mentioned this pull request Oct 8, 2016

Get I/Os per second of each process #261

Closed

plamenko force-pushed the proc_io branch from 76c9b5a to c5f883b Compare October 12, 2016 16:29

dbwiddis merged commit e51f8e8 into oshi:master Oct 13, 2016

dbwiddis mentioned this pull request Dec 20, 2016

3.3 Release TODO #280

Closed

dbwiddis added this to the 3.3 milestone Dec 22, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[OSHI] Add bytesRead and bytesWritten to OSProcess #262

[OSHI] Add bytesRead and bytesWritten to OSProcess #262

plamenko commented Oct 7, 2016

codecov-io commented Oct 7, 2016 •

edited

Loading

coveralls commented Oct 7, 2016

dbwiddis left a comment

dbwiddis Oct 7, 2016 •

edited

Loading

plamenko Oct 12, 2016

dbwiddis Oct 7, 2016

dbwiddis Oct 7, 2016

dbwiddis commented Oct 11, 2016

plamenko commented Oct 12, 2016

plamenko commented Oct 12, 2016

coveralls commented Oct 12, 2016

plamenko commented Oct 12, 2016

coveralls commented Oct 12, 2016

[OSHI] Add bytesRead and bytesWritten to OSProcess #262

[OSHI] Add bytesRead and bytesWritten to OSProcess #262

Conversation

plamenko commented Oct 7, 2016

codecov-io commented Oct 7, 2016 • edited Loading

Current coverage is 84.46% (diff: 100%)

coveralls commented Oct 7, 2016

dbwiddis left a comment

Choose a reason for hiding this comment

dbwiddis Oct 7, 2016 • edited Loading

Choose a reason for hiding this comment

plamenko Oct 12, 2016

Choose a reason for hiding this comment

dbwiddis Oct 7, 2016

Choose a reason for hiding this comment

dbwiddis Oct 7, 2016

Choose a reason for hiding this comment

dbwiddis commented Oct 11, 2016

plamenko commented Oct 12, 2016

plamenko commented Oct 12, 2016

coveralls commented Oct 12, 2016

plamenko commented Oct 12, 2016

coveralls commented Oct 12, 2016

codecov-io commented Oct 7, 2016 •

edited

Loading

dbwiddis Oct 7, 2016 •

edited

Loading