On Wed, 12 Jan 2022 15:18:46 GMT, Tyler Steele <d...@openjdk.java.net> wrote:

>> Just in time for the holidays I have completed an implementation of the JFR 
>> functionality for AIX. As a side note, this is my first submission to 
>> OpenJDK đź‘‹
>> 
>> ### Implementation notes and alternatives considered
>> 
>> After modifying the build system to allow the --enable-jvm-feature-jfr to 
>> work on AIX, my task was to implement the interfaces from os_perf.hpp. The 
>> os_perf_aix.cpp implementation that existed was, I believe, a copy of the 
>> Linux implementation. A review of the code in that file showed that 
>> NetworkInterface, CPUPerformanceInterface, and SystemProcessInterface would 
>> require modification to work on AIX. Using the Linux implementation as a 
>> guide, I initially expected to use files from the aix procfs like 
>> /proc/<pid>/psinfo, and /proc/<pid>/status in a manner similar to the Linux 
>> implementation. However, I ended up using libperfstat for all functionality 
>> required by the interfaces.
>> 
>> ### Testing
>> 
>> Testing for JFR seems to be quite light in the repo both before and after 
>> this change. In addition to performing manual testing, I have added some 
>> basic sanity checks that ensure events can be created and committed (using 
>> jtreg), and performs some basic checks on the results of the interface 
>> member functions (using gtest).
>> 
>> ### More notes
>> 
>> I've sent an email to the JFR group about a TOC overflow warning I 
>> encountered while building (for the release server target). I believe the 
>> fix is to pass -qpic=large when using the xlc toolchain, but my 
>> modifications to flags-cflags.m4 have not been successful in removing this 
>> warning.
>
> Tyler Steele has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains five commits:
> 
>  - Merge branch 'master' into JDK-8203290
>  - Clean up & testing
>    
>    - Run JFR tests in test/jdk/jdk/jfr
>    - Fix issues found from above test suite
>    - Remove unecessary jtreg & gtest tests
>    - Remove ineffective `qpic=large`
>    - TODO: jdk/jfr/event/runtime/TestNativeLibrariesEvent.java
>  - Merge branch 'JDK-8203290' of github.com:backwaterred/jdk into JDK-8203290
>  - 8203290: Implements Java Flight Recorder on AIX
>    
>     - changes build system to allow jfr feature on aix
>     - implements NetworkPerformance, CPUPerformance, and SystemProcess
>     interfaces from os_perf.hpp
>     - implements jfr sanity tests
>  - 8203290: Implements Java Flight Recorder on AIX
>    
>     - changes build system to allow jfr feature on aix
>     - implements NetworkPerformance, CPUPerformance, and SystemProcess
>     interfaces from os_perf.hpp
>     - implements jfr sanity tests

Hi Tyler,

nice to see this effort. I'm curious, are you associated with IBM?

This is not a full review, but I can take a closer look over the next days if 
necessary (but looks like you already got your full set of reviewers).

About ThreadCrashProtection, we unified that into os_posix.cpp, so it should 
work out of the box on any Posix system. Its the usual sigsetjmp/siglongjmp 
game, covering SIGBUS and SIGSEGV. I am not aware of any AIX peculiarities 
which would prevent this from working on AIX.

Cheers, Thomas

make/autoconf/flags-cflags.m4 line 421:

> 419:       # so for debug we build with '-qpic=large -bbigtoc'.
> 420:       DEBUG_CFLAGS_JVM="-qpic=large"
> 421:     fi

Why this removal? Note that getting the TOC not to explode on AIX has been an 
ongoing struggle, see this string of associated JBS issues: 

https://bugs.openjdk.java.net/browse/JDK-8184344
https://bugs.openjdk.java.net/browse/JDK-8171408
https://bugs.openjdk.java.net/browse/JDK-8196488
https://bugs.openjdk.java.net/browse/JDK-8204935

-------------

PR: https://git.openjdk.java.net/jdk/pull/6885

Reply via email to