-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CopyableThreadContextElement implementation #3227
Conversation
…otential regression hitter
PTAL, especially at how clear the documentation (both public and internal) is. I've measured the performance impact and for a classic ping-pong application an additional context scan is negligible and practically undetectable. It neither means that it's zero nor that we won't encounter any issues with it in the future. The biggest potential impact is our future selves -- I can easily imagine 30-50+ elements context in enterprise-grade applications, but it requires a completely different solution, namely rework of the context elements themselves (now being a straightforward immutable map is not really the fastest solution, especially with weird |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't look at the tests yet, but I did try breaking the code in a few non-obvious places to ensure that these are tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked through the tests as well.
kotlinx-coroutines-core/jvm/test/ThreadContextMutableCopiesTest.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
…t.kt Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
* A coroutine using this mechanism can safely call Java code that assumes it's called using a | ||
* `Thread`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the meaning here. Is it possible to call any Java code without using a Thread
?
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
New approach eagerly copies corresponding elements to avoid accidental top-level reuse and also provides merge capability in case when an element is being overwritten. Merge capability is crucial in tracing scenarios to properly preserve the state of linked thread locals Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
No description provided.