util
util copied to clipboard
twitter
java 11 reflect warning fixes
Problem
See https://github.com/twitter/util/issues/266, the current code throws warnings under jdk11
Solution
In jdk11 it is possible to avoid the illegal reflection by simply calling getters that now exist. I tried to stay java 8 backwards compatible by catching a possible NoSuchMethodError. The new code does need to be compiled using jdk11 however. Also its kinda hard to properly test this against jdk8.
Result
The warnings disappear in jdk11. Some additional (manual?) tests are required to check the jdk8 backwards compatibility.
Hi @sjoerd-vogel! Thank you very much for submitting this pull request for util. Unfortunately, it's not going to be something that we can accept at the moment, because we are still using JDK/JVM 8 at Twitter. Hopefully that is resolved early next year. Until then, it means that we cannot call '.getVMManagement' directly.
Would it resolve the warnings you are seeing if the code was changed based on version to conditionally call '.getVMManagement' via reflection?
Hi @sjoerd-vogel! Thank you very much for submitting this pull request for util. Unfortunately, it's not going to be something that we can accept at the moment, because we are still using JDK/JVM 8 at Twitter. Hopefully that is resolved early next year. Until then, it means that we cannot call '.getVMManagement' directly.
Would it resolve the warnings you are seeing if the code was changed based on version to conditionally call '.getVMManagement' via reflection?
I dont think so. AFAIK you get this warning for any reflection on certain classes. Which is why the cast to the anonymous trait was problematic.
If there is anything I can do to speed up the migration of this module to jdk11 I would be happy to help. AFAIK you only need this module to compile against jdk11, I dont think you need to involve the other projects for this and you should still be backwards compatible for running this on older JVMs.
If there is anything I can do to speed up the migration of this module to jdk11 I would be happy to help. AFAIK you only need this module to compile against jdk11, I dont think you need to involve the other projects for this and you should still be backwards compatible for running this on older JVMs.
For our open source code such as this (Util, Finagle, Finatra, TwitterServer, Scrooge), it needs to work both internally and externally. Internally we have a monorepo with source dependencies, and so the cost must work with JDK 8 while the company is running JDK 8. So until we've migrated to JDK11 internally, which should happen sometime early next year, we don't have the ability to use any JDK code that does not compile with JDK 8.
Thank you for the offer to help with this. Unfortunately, my current understanding is that the vast majority of issues are in internal Twitter code.
@ryanoneill can you give us an update when this pull request can be merged?
