retrofit icon indicating copy to clipboard operation
retrofit copied to clipboard

Replace use of synchronized in OkHttpCall with ReentrantLock

Open bendb-instacart opened this issue 10 months ago • 2 comments

We use Retrofit in our backend, and I notice that retrofit2.OkHttpCall relies extensively on synchronized(this), which can pin host threads (when using virtual threads). Would you be open to a PR replacing use of synchronized here with use of ReentrantLock?

bendb-instacart avatar Feb 04 '25 06:02 bendb-instacart

It might be better to replace it with a compare and swap loop with an atomic? We're not guarding an operation, just performing creation where only a single instance can win. Plus the idea of contention here is near zero, and the cost of double allocation before a winner is determined is negligible.

JakeWharton avatar Feb 04 '25 23:02 JakeWharton

Thanks for the pointer - I'll take a look!

bendb-instacart avatar Feb 05 '25 05:02 bendb-instacart