-
Notifications
You must be signed in to change notification settings - Fork 362
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
ThreadUtil: don't use thread APIs if multithreading&commix aren't in use #3884
base: main
Are you sure you want to change the base?
Conversation
INLINE | ||
bool mutex_init(mutex_t *ref) { return true; } | ||
|
||
INLINE bool mutex_lock(mutex_t *ref) { return true; } | ||
|
||
INLINE bool mutex_unlock(mutex_t *ref) { return true; } | ||
#else |
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.
question: I only needed these three so I implemented only these. Perhaps we should do this on a per-function basis?
#if !(defined SCALANATIVE_GC_COMMIX || \ | ||
defined SCALANATIVE_MULTITHREADING_ENABLED) |
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.
thought: it'd be good to have a single definition that we can check, e.g. "has pthread" - does something like this exist already?
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.
also, is this fine or would you rather I flip the boolean (concurrent case first, fallback second)?
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.
By default, all the shared
code should be compiled so something is not correct 100%. The SCALANATIVE_GC_COMMIX
is defined only when selecting Commix so that code will get compiled and similar for other GCs. Does this help at all?
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.
It is used by all the internal GCs and also single threaded runtime because no guard against.
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.
hmm yeah. What I did here was inspired by Wojciech's comment on the ticket, but I see that this might not be the best way to do it because it's not really "shared" anymore.
Closes #3877