Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disabled Windows performance collection leads to empty Process cache #542

Merged
merged 1 commit into from
Jul 5, 2018
Merged

Disabled Windows performance collection leads to empty Process cache #542

merged 1 commit into from
Jul 5, 2018

Conversation

MarcMil
Copy link
Contributor

@MarcMil MarcMil commented Jul 5, 2018

… to an empty cache

The empty cache caused an empty process list.

@MarcMil
Fixes a bug where on Windows, an disabled performance collection lead…
6f8fa91 
… to an empty cache

The empty cache caused an empty process list.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.445% when pulling 6f8fa91 on MarcMil:bugfix-emptycache into c86750a on oshi:master.

@codecov-io
Copy link

Codecov Report

Merging #542 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #542   +/-   ##
=========================================
  Coverage     84.23%   84.23%           
  Complexity      194      194           
=========================================
  Files            28       28           
  Lines          1427     1427           
  Branches        208      208           
=========================================
  Hits           1202     1202           
  Misses          105      105           
  Partials        120      120

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c86750a...6f8fa91. Read the comment docs.

@dbwiddis dbwiddis changed the title Fixes a bug where on Windows, an disabled performance collection lead… Disabled Windows performance collection leads to empty Process cache Jul 5, 2018
@dbwiddis dbwiddis merged commit df5f518 into oshi:master Jul 5, 2018
@dbwiddis
Copy link
Member

dbwiddis commented Jul 5, 2018

Thanks for this fix! A question -- if the performance data is disabled is there any other backup source for the same information (e.g., WMI) or does WMI draw from the same source eventually (I think it does)?

@dbwiddis
Copy link
Member

dbwiddis commented Jul 5, 2018

Also is this a one-time fix of an empty cache, or does the disabled performance data cause the cache to be cleared every time?

@MarcMil
Copy link
Contributor Author

MarcMil commented Jul 6, 2018 •
edited
Loading

Actually, I did not check any WMI source. However, when I run the Unit testcases on my computer, it semes to fail to get the command line arguments via WMI.
For my use case I did not rely on such additional information, I just wanted to know whether a process with a specific process id was still running and I was confused when it returned null although the process under watch was still running.
On my machine, this is the result when querying the Performance Data:

WinPerf$PERF_DATA_BLOCK(allocated@0xca9870 (88 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes))))) {
  char Signature[4]@0=[C@2846
  int LittleEndian@8=1
  int Version@c=1
  int Revision@10=1
  int TotalByteLength@14=78
  int HeaderLength@18=78
  int NumObjectTypes@1c=0
  int DefaultObject@20=ee
  WinBase$SYSTEMTIME SystemTime@24=WinBase$SYSTEMTIME(allocated@0xca9894 (16 bytes) (shared from allocated@0xca9870 (88 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes)))))) {
    short wYear@0=7e2
    short wMonth@2=7
    short wDayOfWeek@4=4
    short wDay@6=5
    short wHour@8=b
    short wMinute@a=2e
    short wSecond@c=23
    short wMilliseconds@e=44
  }
  WinNT$LARGE_INTEGER PerfTime@38=WinNT$LARGE_INTEGER(allocated@0xca98a8 (8 bytes) (shared from allocated@0xca9870 (88 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes)))))) {
    WinNT$LARGE_INTEGER$UNION u@0=WinNT$LARGE_INTEGER$UNION(allocated@0xca98a8 (8 bytes) (shared from allocated@0xca98a8 (8 bytes) (shared from allocated@0xca9870 (88 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes))))))) {
      WinNT$LARGE_INTEGER$LowHigh lh@0=WinNT$LARGE_INTEGER$LowHigh(auto-allocated@0x18fdd770 (8 bytes)) {
        WinDef$DWORD LowPart@0=0
        WinDef$DWORD HighPart@4=0
      }
      long value@0=3696847d49
    }
  }
  WinNT$LARGE_INTEGER PerfFreq@40=WinNT$LARGE_INTEGER(allocated@0xca98b0 (8 bytes) (shared from allocated@0xca9870 (88 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes)))))) {
    WinNT$LARGE_INTEGER$UNION u@0=WinNT$LARGE_INTEGER$UNION(allocated@0xca98b0 (8 bytes) (shared from allocated@0xca98b0 (8 bytes) (shared from allocated@0xca9870 (88 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes))))))) {
      WinNT$LARGE_INTEGER$LowHigh lh@0=WinNT$LARGE_INTEGER$LowHigh(auto-allocated@0x18fdd510 (8 bytes)) {
        WinDef$DWORD LowPart@0=0
        WinDef$DWORD HighPart@4=0
      }
      long value@0=28697a
    }
  }
  WinNT$LARGE_INTEGER PerfTime100nSec@48=WinNT$LARGE_INTEGER(allocated@0xca98b8 (8 bytes) (shared from allocated@0xca9870 (88 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes)))))) {
    WinNT$LARGE_INTEGER$UNION u@0=WinNT$LARGE_INTEGER$UNION(allocated@0xca98b8 (8 bytes) (shared from allocated@0xca98b8 (8 bytes) (shared from allocated@0xca9870 (88 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes) (shared from allocated@0xca9870 (8192 bytes))))))) {
      WinNT$LARGE_INTEGER$LowHigh lh@0=WinNT$LARGE_INTEGER$LowHigh(auto-allocated@0x18fdd620 (8 bytes)) {
        WinDef$DWORD LowPart@0=0
        WinDef$DWORD HighPart@4=0
      }
      long value@0=1d41455d3514f56
    }
  }
  int SystemNameLength@50=1c
  int SystemNameOffset@54=58
}

As you can see, NumObjectTypes is 0 and therefore the cache is completely empty. The fix does account for exactly these cases, where the cache cannot fulfill its purpose.

@dbwiddis
Copy link
Member

dbwiddis commented Jul 6, 2018

The fix does solve one problem, but it creates some inefficiencies. The cache is emptied at the end of updateProcessMapFromRegistry() in a one-by-one manner (vs. clear()) and then entirely rebuilt.

It may be preferable to try to preserve some of the objects in the cache in your case, although admittedly it may not be worth the effort for this edge case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MarcMil @coveralls @codecov-io @dbwiddis