Skip to content
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

remove unncessary Current_thread restore in caml_thread_enter_blocking_section #11019

Merged
merged 1 commit into from Feb 17, 2022

Conversation

sadiqj
Copy link
Contributor

@sadiqj sadiqj commented Feb 16, 2022

Identified by @gadmm in #10966 (comment) it seems we're doing an unnecessary restore of Current_thread in Systhreads' caml_thread_enter_blocking_section.

4.14's caml_thread_enter_blocking_section for reference: https://github.com/ocaml/ocaml/blob/4.14/otherlibs/systhreads/st_stubs.c#L230

cc @Engil

@abbysmal
Copy link
Contributor

This change looks good to me,
As mentioned in #10966, I think this is a leftover from the early reimplementation of systhread on Multicore.
I cannot trace it to anything meaningful, and considering caml_thread_enter_blocking_section can only be called from a thread already holding the lock (meaning, Current_thread should be set to the executing thread.), I think this change is fine.

@abbysmal abbysmal merged commit 210f41f into ocaml:trunk Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants