Add SIMD support for Java 21 #90
Conversation
Add changed proposed from pufferfish-gg#81
Add changes proposed from pufferfish-gg#81
|
Oops misclick! Just gonna do my actual review now |
There was a problem hiding this comment.
JEPs 460 (targeting JDK 22) and 469 (targeting JDK 23) specify nothing which'd break the current patches - although Vector API is incubator and not preview I'd say it's safe enough to assume that the API won't change substantially enough to warrant restricting things here (reduces unnecessary friction each time there's a JDK release). If things do (by unlikely chance) change enough then we could always reimplement the other end of the check (assuming these suggestions are accepted).
| @@ -48,7 +48,7 @@ index 0000000000000000000000000000000000000000..ab5fea0b03224bf249352ce340e94704 | |||
| + @Deprecated | |||
| + public static boolean canEnable(Logger logger) { | |||
| + try { | |||
| + if (SIMDDetection.getJavaVersion() != 17 && SIMDDetection.getJavaVersion() != 18 && SIMDDetection.getJavaVersion() != 19) { | |||
| + if (SIMDDetection.getJavaVersion() < 17 || SIMDDetection.getJavaVersion() > 21) { | |||
There was a problem hiding this comment.
| + if (SIMDDetection.getJavaVersion() < 17 || SIMDDetection.getJavaVersion() > 21) { | |
| + if (SIMDDetection.getJavaVersion() < 17) { |
| @@ -42,15 +42,15 @@ index deb52c185705c4b4186c7bae02f2a827620c20ca..d1a8bfb29817d2b73689713b4d5f04d7 | |||
| + // Attempt to detect vectorization | |||
| + try { | |||
| + SIMDDetection.isEnabled = SIMDDetection.canEnable(PufferfishLogger.LOGGER); | |||
| + SIMDDetection.versionLimited = SIMDDetection.getJavaVersion() != 17 && SIMDDetection.getJavaVersion() != 18 && SIMDDetection.getJavaVersion() != 19; | |||
| + SIMDDetection.versionLimited = SIMDDetection.versionLimited = SIMDDetection.getJavaVersion() < 17 || SIMDDetection.getJavaVersion() > 21; | |||
There was a problem hiding this comment.
| + SIMDDetection.versionLimited = SIMDDetection.versionLimited = SIMDDetection.getJavaVersion() < 17 || SIMDDetection.getJavaVersion() > 21; | |
| + SIMDDetection.versionLimited = SIMDDetection.versionLimited = SIMDDetection.getJavaVersion() < 17; |
There was a problem hiding this comment.
No. We have been over this in the past.
| + } catch (NoClassDefFoundError | Exception ignored) { | ||
| + ignored.printStackTrace(); | ||
| + } | ||
| + | ||
| + if (SIMDDetection.isEnabled) { | ||
| + PufferfishLogger.LOGGER.info("SIMD operations detected as functional. Will replace some operations with faster versions."); | ||
| + } else if (SIMDDetection.versionLimited) { | ||
| + PufferfishLogger.LOGGER.warning("Will not enable SIMD! These optimizations are only safely supported on Java 17, Java 18, and Java 19."); | ||
| + PufferfishLogger.LOGGER.warning("Will not enable SIMD! These optimizations are only safely supported on Java 17 through Java 21."); |
There was a problem hiding this comment.
| + PufferfishLogger.LOGGER.warning("Will not enable SIMD! These optimizations are only safely supported on Java 17 through Java 21."); | |
| + PufferfishLogger.LOGGER.warning("Will not enable SIMD! These optimizations are only safely supported on Java 17 and higher."); |
|
@CyberFlameGO This feature is still actively being modified. Due to the performance-sensitive nature of this, we MUST actually verify that the machine code generated by the JIT is actually faster. If anything is broken or changed in the implementation of the JIT, it could substantially alter the performance characteristics of this patch. This is why we need thorough review for EVERY release. This is not as simple as just enabling for all java versions, or even just enabling for one specific java version. And the procedure to validate this correctly is complex. None of the "reviewers" that have reviewed this have actually reviewed the correct thing. |
|
Ahhh my bad - I was just going off of the historical summary from the JEPs I linked |
Fixes #81