Message-ID: From: "Sanne (@Sanne)" To: "pgjdbc/pgjdbc" Date: Mon, 30 Jun 2025 09:13:21 +0000 Subject: Re: [pgjdbc/pgjdbc] issue #3693: Overhead of uncontended use of ResourceLock In-Reply-To: References: List-Id: X-GitHub-Author-Login: Sanne X-GitHub-Comment-Id: 3018396581 X-GitHub-Comment-Type: issue_comment X-GitHub-Issue: 3693 X-GitHub-Repo: pgjdbc/pgjdbc X-GitHub-Type: comment X-GitHub-Url: https://github.com/pgjdbc/pgjdbc/issues/3693#issuecomment-3018396581 Content-Type: text/plain; charset=utf-8 Hi @davecramer ! I'll explore something then, thanks! Hi @vlsi , thank you very much for the comments! I need to look at the code with some care to understand your scenario better. At first impression I'm not really sure I sympatise with a user expecting a very specific behaviour from a concurrent, uncoordinated call to `getWarnings` ? Surely they can't be relying of going to get the set of warnings as defined before or after another operation when there is no coordination on their side, such as to ensure that the `getWarnings` invocation itself is happening before rather than after the other operation? But I'm not sure of such use cases, I'll think about it - thank you so much for the point. I am totally aware that simplifying the locking schemes will be tricky and perhaps I should stay away from that. > however https://github.com/pgjdbc/pgjdbc/issues/1951#issuecomment-1254442789, the understanding was that ReentrantLock was cheap enough. Right I've seen the previous discussion. I actually remember it .. we had to patch Hibernate for loom compatibility as a follow up :) But in this particular benchmark I'm running these days it's noticeable. Again, I admit it's not the main concern we have - there's other things in the picture that I'd also want to improve on, but this one seems like a low hanging fruit as we're saturating CPU and this is consuming about 2% CPU for no useful reason (in our case). If I had the flag to fully disable it, we'd also save on some allocations which is also always welcome. > We could indeed replace certain lock instances with synchronized (e.g. LruCache) Slightly off-topic, but I'd also love it if the cache implementation could be pluggable: I'm sure we'd not want pgjdbc to bring in additional dependencies, yet it's common for various runtime platforms (including Quarkus) to have Caffeine or other state of the art cache implementations around. It would be nice if I could have pgjdbc make use of it, similarly to what I did in https://hibernate.atlassian.net/browse/HHH-19544 . I might open an issue about that later, as I don't have evidence of that being useful yet at this point. > Benchmarks are always welcome. If you could contribute a benchmark (Quarkus + Hibernate is fine) or a link to the benchmark, that would be great. We have such benchmarks in Techempower, publicly available: - https://github.com/Sanne/FrameworkBenchmarks/tree/master/frameworks/Java/quarkus It's been there for some years - the reason I'm bringing this particular issue up only now is that previously this particular cost was just a "drop in the bucket", but as we evolved Quarkus, Hibernate, Vert.x, Netty and the JDK and other bottlenecks are being removed, this one which was a negligible detail in the past is starting to become more obvious. I'm also working on an update PR for Techempower which is pushing figures much higher still, hopefully this fix could be a part of that batch of improvements: essentially I want the Java leading platforms to be able to deliver great results when compared to other programmig languages and platforms, any help is welcome :) Attaching a new flamegraph zip file, hopefully this works better. [flamegraph_quarkus_db_256.zip](https://github.com/user-attachments/files/20976538/flamegraph_quarkus_db_256.zip)