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

Add mappings for AIX Perfstat library #1217

Merged
merged 1 commit into from
Jul 19, 2020

Conversation

dbwiddis
Copy link
Contributor

@dbwiddis dbwiddis commented Jul 5, 2020

Adds mappings to several structures and functions in AIX Perfstat library. Documentation at https://www.ibm.com/support/knowledgecenter/ssw_aix_71/performancetools/idprftools_perfstat.html

@dbwiddis
Copy link
Contributor Author

dbwiddis commented Jul 5, 2020

For ease of review:

  • Structure mapping is mostly straightforward. Most native u_longlong_t mapped directly to long.
  • The perfstat_disk_t native header included a comment "Pad of 3 short is available here" after the wpar_id field. I manually added the appropriate padding, but it's possible that there's a better solution using structure alignment.
  • The union in the perfstat_protocol_t structure should be carefully reviewed for correctness. I have manually verified sane outputs for the more common protocols, but a unit test can not assume any particular value is nonzero. So beyond testing the higher-level fields, I'm not sure how to unit-test correct union behavior
  • Exceptions to the mappings include perfstat_dktype_t and perfstat_partition_type_t which are 32-bit unions supporting bitfields; I simplified using int types.
  • The lbolt field in the perfstat_cpu_total_t structure is type time_t. I mapped this to NativeLong based on the relevant section of the header in /usr/include/sys/types.h:
#ifdef  _LINUX_SOURCE_COMPAT
typedef long int        time_t;
#else   /* !_LINUX_SOURCE_COMPAT */
#ifdef __64BIT__
typedef long            time_t;
#else
typedef int             time_t;
#endif

You may find it convenient to inspect the contents of the structures using util classes on this project which were the basis for the test cases and are producing "sane" output.

I did update the build.xml script to include the new package in the osgi modules. Let me know if there's any other place that needs updating.

@dbwiddis
Copy link
Contributor Author

dbwiddis commented Jul 5, 2020

And, apparently I can't use the static method to load the interface in Java 6, so I'll have to think about a better way to do that. Suggestions welcome.

Copy link
Member

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, with minimal comment inline. Did you manage to run the unittests on aix?

@dbwiddis
Copy link
Contributor Author

Deleted the pad and everything still works (exact copies of the files from this PR in my own project, with only package name/imports changed).

[INFO] Running oshi.jna.platform.unix.aix.PerfstatTest
[INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.669 s - in oshi.jna.platform.unix.aix.PerfstatTest

Will make the other requested changes.

@dbwiddis
Copy link
Contributor Author

One other note: the separate shared object loader class is necessary on Java 6, but a static interface method works in Java 8. It's also a potential solution to update NativeLibrary#loadLibrary with an AIX branch to check shr.o or shr_64.o in case the .a loading fails.

@matthiasblaesing
Copy link
Member

One other note: the separate shared object loader class is necessary on Java 6, but a static interface method works in Java 8. It's also a potential solution to update NativeLibrary#loadLibrary with an AIX branch to check shr.o or shr_64.o in case the .a loading fails.

The aix optimization could be done later. For now it would be good to make SharedObjectLoader package private, that way it is not exposed outside jna-platform and we are free to change it as necessary.

@dbwiddis
Copy link
Contributor Author

License added and shared loader made package private. Should be ready to merge.

Copy link
Member

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sane. Thank you.

@dbwiddis dbwiddis merged commit bb8f286 into java-native-access:master Jul 19, 2020
@dbwiddis dbwiddis deleted the aix branch July 19, 2020 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants