search

square / okio

A modern I/O library for Android, Java, and Kotlin Multiplatform.
https://square.github.io/okio/
Apache License 2.0
8.77k stars 1.18k forks source link

Use a state machine to implement AsyncTimeout #1397

Closed swankjesse closed 9 months ago

swankjesse commented 9 months ago

Previously we had 3 boolean state variables to track 4 possible states. That was more complex than necessary.

yyaroshevich commented 4 months ago

Hello,

I'm not sure if this is the right place to ask, but I hope @swankjesse, as the author of several timeout-related PRs, might be able to help. Please know that I ask this question with the utmost respect and curiosity, and it is not meant to be critical or blaming in any way.

Is there a specific reason why AsyncTimeout is still implemented as a manual linked list with a backing thread instead of using ScheduledThreadPoolExecutor in 2024? I understand that this class was initially written before JDK 8, but now it seems that much of the complex code could be simplified by using ScheduledThreadPoolExecutor.

I'm asking because I've observed that list manipulations, such as linear time insertions into the linked list, have become a bottleneck in a back-end service that heavily relies on OkHttpClient and probably it is bottleneck in other apps which heavily uses OkHttpClient.

Thank you for any insights you can provide.

Best regards, Yury

swankjesse commented 4 months ago

@yyaroshevich that’s a good point!

Wanna open a tracking bug for us to consider switching? At the time of creation we optimized for fewest allocations, which were particularly slow on Android.

One other option that we should investigate – could we accomplish better performance by searching backwards instead of forwards in the list? If all of the timeouts are symmetric (they often are), then inserts will usually be at the end. We could reduce contention without increasing allocation?

yyaroshevich commented 4 months ago

Wanna open a tracking bug for us to consider switching?

Sure, I've opened ticket where we can proceed discussion https://github.com/square/okio/issues/1488. Thanks a lot!