Changes to Bellsoft/Marvell method of developing intrinsics
derekw at marvell.com
Wed Jan 23 17:27:34 UTC 2019
First I should describe the relationship between myself, Marvell, and Bellsoft. I'm the JVM team lead at Marvell/Cavium, and we work as a virtual team with Bellsoft to help port, analyze, and optimize the aarch64 port of OpenJDK (as well as Hadoop, etc). Bellsoft also contributes to OpenJDK independently.
Andrew Dinn has brought up several good points on testing, code quality, and when and where code complexity should be spent in the aarch64 port. I'll describe my general thoughts on code complexity, what Bellsoft does generally for testing before check-ins, as well as describe what we will be doing for new and existing complex intrinsics code.
Intrinsics are a category of code that can handle more complexity than usual because the complexity is quite local. A developer can generally ignore the details hiding in the implementation unless actively reviewing or enhancing the intrinsic. But while pockets of complexity are OK, black holes of complexity are not. The effort to understand the intrinsics must be substantially less than then effort to develop it. The nature of intrinsics also make them easier to test in isolation, but the testing has to be sufficient. And I agree that the performance gain of each intrinsic has to justify the work developing and supporting it.
Bellsoft's current testing process, before sending a patch for review, is developing testing specific to the patch itself and testing for regressions with JCK and relevant jtreg tests. If the patch is in shared code, it undergoes testing on Linux x86, ARM, AARCH64, Windows, Mac, Solaris x86 and SPARC.
Obviously this has not been sufficient to prevent bugs in the more complex intrinsics we've implemented for aarch64 - even with the stellar code review provided by the community. And the effort required to review the intrinsics has been too high.
Because of this we will change how we develop patches for complex intrinsics. Before sending the code out for public review, we intend to:
* Use an additional "red-team" developer to focus on finding the weak points in the code and develop tests that ensure code coverage testing, test case coverage, etc. This is in addition to the normal testing and test development that the initiating developer is expected to do.
* The "red-team" developer will also suggest changes for code clarity and code documentation, and will document the test strategy (what cases are tested, what tests cover what code, how to run tests).
* We will include all tests developed as part of the patch, even if some modes may not be practical to run regularly as jtreg tests (for example if some tests take excessive time). This will allow later enhancements or fixes to the intrinsic to go through at least as thorough testing as the original.
By breaking the patch development task into two roles we expect to end up with better code quality and make the reviewing task easier.
Note that this is the process that we will be using. We don't expect the rest of the community to adopt this, or if they did, agree on exactly how complex a "complex intrinsic" needs to be to warrant this approach.
We will also begin back-reviewing existing complex intrinsics. If other members of the community are interested in working on this we can coordinate to ensure coverage.
Please let me know if you have any comments on this plan. Thanks,
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-compiler-dev