Even if you're not on Java 21, this change may affect allocation and GC rates, so feedback is welcome if that's something you are able to A/B test

Tested with bb and ecosystem libs, no problems found so far:

{:test 2356, :pass 14713, :fail 0, :error 0}

Pushed it into our CI. If there are no problems we'll run it on QA for a bit, then push to production. We're on JDK 20 right now (awaiting official JDK 21 support from New Relic before we can update) but we have --enable-preview and could also test vthreads...

Passes all our tests so it's on our QA system now.

For those testing, you may want to check performance as well if you have portions of your code where that is an important concern. I mention this because I was reading about this a bit more and one article I read said their performance testing showed between 2 - 7.5x performance difference between synchronized and locks. It may be just that their test was simple or whatnot, but I thought it may be worth mentioning. The performance difference may be offset by the gains you get via virtual threads as well. Additionally, we all know that performance tends to be problem specific and such as well. @U064X3EF3 probably has better info on what they see in regards to performance with locks.

That does not match what we found in testing but it is definitely complicated, and varies significantly depending on jdk

Thanks Alex. I knew you’d know more

In Java 21, we found generally that lock/unlock was significantly faster with RL but reentrant locking (which we are no longer doing) was slower. There is a cost to allocation though and generally I think the seq realization is a bit slower, but seq walking is now faster (due to refactoring to streamline the realized path)

But those numbers vary on older jvms, particularly pre-15 with biased locking

In summary, it is really complicated and hard to predict exactly how it affects any particular application, so please test!

> reentrant locking (which we are no longer doing) was slower. > Could you clarify/elaborate on this? The change in the code seems to be replacing synchronized with use of a ReentrantLock...

Prior, seq() called sval(), both were synchronized so you reenter the lock (lock something already locked), that never happens now. Reentrant locking with synchronized seems basically free but reentrant locking with RL seems to still have some costs with unlock signaling

Thanks Alex. That makes it clearer.

Tbh, it is kind of shocking that virtual threads went out without consideration of all this. Lots of Java code just assumes reentrant synchronization is cheap/free

I wonder if fewer people tested them in preview in real world situations than they expected? It seems that they knew about the locking/pinning issues but considered them minor...

From the JEP “The scheduler does not compensate for pinning by expanding its parallelism. Instead, avoid frequent and long-lived pinning by revising synchronized blocks or methods that run frequently and guard potentially long I/O operations to use java.util.concurrent.locks.ReentrantLock instead. There is no need to replace synchronized blocks and methods that are used infrequently (e.g., only performed at startup) or that guard in-memory operations. As always, strive to keep locking policies simple and clear.” Still not that clear and their solution is ReentrantLocks, which as Alex pointed out seems to still have issues. Seqs would seem to be in memory guarded objects wouldn’t they? Would they consider our “user code” frequent or long lived? I guess so since in Clojure we use seqs for so much. Regardless, thanks to the team for bringing this capability to the Clojure Community. One more amazing reason that using Clojure is so amazing.

This comment on StackOverflow seems to talk about the implementation a bit. https://stackoverflow.com/a/76788835

Thanks again for the info Alex

Just found this from an August of this year. Maybe they mention some of these things. I’m gonna watch, and thought you all may enjoy it as well. https://youtu.be/WsCJYQDPrrE?si=S401YxgC8ocNNwue

> Would they consider our “user code” frequent or long lived? Since this is user code, it can be anything, so it must be presumed that it can be long lived (certainly also true for delay)

There are a number of other uses of synchronized in Clojure but most of those are protecting field access (short lived) and we have no plans to modify those, they should be fine.

that talk is a good overview. another related issue you might ask about after watching that is ThreadLocals and I have another small change for that queued up in https://clojure.atlassian.net/browse/CLJ-2803, but I don't think it's critical

Thanks again for chatting about this. I learned quite a bit about this stuff from investigating this and all the comments.