Thanks for all of the work on this, Ekaterina! I would add a third point: - If newer JVMs offer a standard way to access things that no longer requires unsafe or native access, we can move to that standard approach once all supported JVMs are in scope
The case I was thinking about specifically was the method to get the PID, since Java 9 introduced a way to do this (although we have to wait until Java 8 is unsupported). As the JVM continues to improve I suspect other areas where we have native or unsafe workarounds will become replaceable. Note that performance would still be a sufficient reason to keep existing native or unsafe access since that's an important requirement. Cheers, Derek On Mon, Mar 13, 2023 at 11:28 AM Ekaterina Dimitrova <e.dimitr...@gmail.com> wrote: > Hi everyone, > To close this thread, I see lazy consensus around JDK internals accesses > as follows: > - we keep the current accesses to JDK internals in the Cassandra codebase, > they were carefully considered during former reviews already. If there is > breakage from changes in JDK internals (which are not guaranteed to provide > backward compatibility) - they will be addressed on a per case basis. Like > the example I provided earlier - CASSANDRA-14173 > - we will keep on being very conservative and carefully consider any new > access to JDK internals being proposed to land in the Cassandra codebase. > We document in tickets what other options were considered to make easier > future maintenance or in case of breakages in order to have context. > Covered also in [1] > > Best regards, > Ekaterina > > [1]https://lists.apache.org/thread/33dt0c3kgskrzqtp4h8y411tqv2d6qvh > > > > On Tue, 7 Mar 2023 at 14:58, Ekaterina Dimitrova <e.dimitr...@gmail.com> > wrote: > >> Thanks Benjamin, please, find below my comments. >> >> "It is not necessarily a problem as long as we do get an issue with the >> Modules boundaries and their access. For me it needs to be looked at on a >> case by case basis." >> >> We can still use the --add-opens/add-exports with Java 17(I mentioned I >> added some as part of introducing experimental JDK17 support - >> CASSANDRA-18258) so the concern is that we should do it as little as >> possible as things can break at any time. JDK internals do not guarantee >> backward compatibility. When something can break is unknown and we need to >> be careful. We also had agreement on that in [1] >> >> "It was used mainly to get around the fact that Java did not offer other >> means to do certain things. Outside of trying to anticipate some of the >> restrictions of that API and make sure that the JDK offers a suitable >> replacement for us. I am not sure that there is much that we can do. But I >> might misunderstand your question." >> I think in some cases it was used for performance, too. >> I think some people in our community might have more history around >> Unsafe. I know there were different conversations in the past between >> members of our community and the JDK community regarding Unsafe replacement >> in time but I cannot find any final outcome so it is an honest question if >> anyone has something more to share here. >> >> Now in some of the cases we use internals I found there is fallback which >> can still have some implications like being slower option. So there are >> nuances and lots of history in decisions taken around the Cassandra >> codebase, as usual. Regarding Unsafe, with JDK17 the only concern is with >> Jamm so far because we do not use ObjectFieldOffset with lambdas >> (implemented internally as hidden classes) in Cassandra code or at least I >> didn't find such a place so far. >> >> Even if we decide to go with - "let's keep things as is we will look into >> breakages in time", there needs to be visibility and awareness and >> consideration at least when new code is added. I've heard different >> opinions on the topic around the community, honestly - whether the code >> should stay as-is and breakages to be addressed on a per case basis or not. >> I do not see us having exact guidance. Thoughts? >> >> [1] https://lists.apache.org/thread/33dt0c3kgskrzqtp4h8y411tqv2d6qvh >> >> On Thu, 2 Mar 2023 at 7:48, Benjamin Lerer <ble...@apache.org> wrote: >> >>> Hey Ekaterina, >>> Thanks for the update and all the work. >>> >>> >>>> -- we also use setAccessible at numerous places. >>> >>> >>> It is not necessarily a problem as long as we do get an issue with the >>> Modules boundaries and their access. For me it needs to be looked at on a >>> case by case basis. >>> >>> - thoughts around the usage/future of Unsafe? History around the choice >>>> of using it in C* and future plans I might not know of? >>>> >>> >>> It was used mainly to get around the fact that Java did not offer other >>> means to do certain things. >>> Outside of trying to anticipate some of the restrictions of that API and >>> make sure that the JDK offers a suitable replacement for us. I am not sure >>> that there is much that we can do. But I might misunderstand your question. >>> >>> Le mer. 1 mars 2023 à 21:16, Ekaterina Dimitrova <e.dimitr...@gmail.com> >>> a écrit : >>> >>>> Hi everyone, >>>> Some updates and questions around JDK 17 below. >>>> First of all, I wanted to let people know that currently Cassandra >>>> trunk can be already compiled and run with J8 + J11 + J17. This is the >>>> product of the realization that the feature branch makes it harder for >>>> working on JDK17 related tickets due to the involvement of too many moving >>>> parts. Agreement reached in [1] that new JDK introduction can be done >>>> incrementally. Scripted UDFs removed, hooks to be added in a follow up >>>> ticket. >>>> What does this mean? >>>> - Currently you can compile and run Cassandra trunk with JDK >>>> 17(further to 8+11). You can run unit and java distributed tests already >>>> with JDK17 >>>> - CASSANDRA-18106 in progress, enabling CCM to handle JDK8, 11 and 17 >>>> with trunk and when that is ready we will be able to run also Python tests; >>>> After that one lands it comes CASSANDRA-18247 ; its goal is to add CircleCI >>>> config (separate of the one we have for 8+11) for 11+17 which can be used >>>> from people who work on JDK17 related issues. Patch proposal already in the >>>> ticket. Final version we will have when we do the switch 8+11 to 11+17, >>>> things will go through evolution. >>>> >>>> What does this mean? Anyone who is interested to test or to help with >>>> JDK17 effort can easily do it directly from trunk. Jenkins and CircleCI are >>>> not switched from 8+11 to 11+17 until we are ready. Only test experimental >>>> additional CircleCI config will be added, temporary to make it easier for >>>> testing >>>> >>>> To remind you - the umbrella ticket for the JDK17 builds is >>>> CASSANDRA-16895. >>>> Good outstanding candidate still not assigned - CASSANDRA-18180, if >>>> anyone has cycles, please, take a look at it. CASSANDRA-18263 might be also >>>> of interest to someone. >>>> >>>> In other news, I added already to the JDK17 jvm options certain >>>> imports/exports which are needed at this point but as we agreed in the past >>>> - it will be good to try to eliminate as many of them as we can. Consider >>>> those experimental in my opinion. >>>> Some of them though are related to: >>>> -- some were added already from 11; thoughts? >>>> *-- *some will be eliminated with some maintenance in progress >>>> *-- *some are related to >>>> https://chronicle.software/chronicle-support-java-17. I guess we are >>>> cornered with those until Chronicle eliminates the need for those. >>>> (CASSANDRA-18049) >>>> -- Find a way to get FileDescriptor.fd and >>>> sun.nio.ch.FileChannelImpl.fd without opening internals ( >>>> CASSANDRA-17850) >>>> -- we also use setAccessible at numerous places. >>>> And I am sure our CI will tell me I am missing something, especially >>>> when trunk is alive... >>>> >>>> A few other questions: >>>> - thoughts around the usage/future of Unsafe? History around the choice >>>> of using it in C* and future plans I might not know of? >>>> - ECJ - It seems the compiler artifacts are moved from here >>>> <https://mvnrepository.com/artifact/org.eclipse.jdt.core.compiler/ecj> >>>> to here <https://mvnrepository.com/artifact/org.eclipse.jdt/ecj> and >>>> there is change of license from EPL1.0 to EPL2.0 too. But if I read >>>> correctly here >>>> <https://www.apache.org/legal/resolved.html#weak-copyleft-licenses> that >>>> should not affect us. I am dealing with this in CASSANDRA-18190. Please let >>>> me know if you see any problem with this that I might be missing. >>>> - Looking at the history of tickets around JMXServerUtils class I guess >>>> it was accepted that we might have breakages (and we already had >>>> CASSANDRA-14173) - JmxRegistry extends sun.rmi.registry.RegistryImpl? >>>> >>>> Best regards, >>>> Ekaterina >>>> >>>> [1] https://lists.apache.org/thread/c39yrbdszgz9s34vr17wpjdhv6h2oo61 >>>> >>>> >>>> -- +---------------------------------------------------------------+ | Derek Chen-Becker | | GPG Key available at https://keybase.io/dchenbecker and | | https://pgp.mit.edu/pks/lookup?search=derek%40chen-becker.org | | Fngrprnt: EB8A 6480 F0A3 C8EB C1E7 7F42 AFC5 AFEE 96E4 6ACC | +---------------------------------------------------------------+
