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.

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

Static maps are inefficient and possibly not thread safe #1106

Closed
dbwiddis opened this issue Feb 13, 2020 · 2 comments · Fixed by #1107
Closed

Static maps are inefficient and possibly not thread safe #1106

dbwiddis opened this issue Feb 13, 2020 · 2 comments · Fixed by #1107
Labels
design discussion Ways to improve OSHI's design performance Speeding up OSHI or reducing memory footprint thread safety Issues impacting thread safety

Comments

@dbwiddis
Copy link
Member

dbwiddis commented Feb 13, 2020

I realized after committing #1101 that the static memoized map I used in MacNetworks, while thread safe, would never be GC'd and thus should not have been static.

FreeBSDDisks has a non-thread-safe mountMap and diskMap.

These should be made thread-safe and instance-based maps, and the codebase should be analyzed for any other static maps or other data caches.

@dbwiddis dbwiddis added performance Speeding up OSHI or reducing memory footprint thread safety Issues impacting thread safety labels Feb 13, 2020
@dbwiddis dbwiddis linked a pull request Feb 14, 2020 that will close this issue
@dbwiddis
Copy link
Member Author

OK, this is actually a bigger design question. What's happening is the result of a design decision when first creating the Networks classes. The way the Networks class is implemented is:

  1. Platform-specific HAL instantiates a new platform-Networks object and calls its getNetworks method.
  2. An abstract class calls core Java methods to get an enumeration of NetworkInterface objects.
  3. The abstract class iterates over these objects to create a NetworkIF object (losing the platform-specific nature)
  4. The updateNetworkStats method is called on each object, internally determining which OS's static update method to use
  5. The static method has only the NetworkIF as an argument so it has no ability to use cached information from a single command that collects data from all the other objects. This forces a choice of running the same method repeatedly for individual interfaces, or saving the state in a static object (as currently implemented). Unfortunately as a static member of the class, it never goes away.

Similar requirements also occur in the collection of HWDiskStore objects and OSProcess objects, but each of these is handled differently. In the case of the disks, the platform-specific Disks classes handle everything internally to the platform specific application, which can be more efficient (e.g., call iostat with the disk specified for single calls, or without it to collect information for all the disks) but duplicates code. For the processes, the platform-specific OperatingSystem classes used to create the processes are passed as objects to the OSProcess object so their non-static methods can be accessed and state preserved, but the objects can be GC'd once the user is done with all the processes.

I'm not sure which of these three approaches is the best, and would appreciate some input from others. I'm also thinking of completely (and consistently) redesigning all of these classes to move in the direction of external data/driver classes, possibly as part of a major version bump.

Thoughts from other collabborators? Examples of someone you think has done it right?

@dbwiddis dbwiddis added the design discussion Ways to improve OSHI's design label Feb 14, 2020
@dbwiddis
Copy link
Member Author

OK, did a refactoring of MacDisks that minimizes the duplication and keeps the efficiency. I'll close this issue with that PR but consider a full redesign of Disks/Networks/Processes with the next major version bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design discussion Ways to improve OSHI's design performance Speeding up OSHI or reducing memory footprint thread safety Issues impacting thread safety
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant