Skip to content

Conversation

@paulirotta
Copy link

Recent changes are tested in Windows. No apparently problems. Excellent!

This PR add a stub for macOS to eliminate the need for runtime platform checks. Instead a specific new exception type is thrown for UnsupportedPlatform while still allowing compilation despite C stdlib differences from Unix standard.

I've assumed that macOS is the exception to platform family "unix". An alternative would be that "linux" is the only platform supporting our threading constructs- not sure, but a reasonable guess which works either way.

Some of the documentation examples needed simplification to avoid "docs are always on Linux, so the old example assertions in docs were not true due to UnsupportedPlatform" when running tests on mac. Thus you likely want to bump the secondary version number for any release.

//! use thread_priority::*;
//!
//! assert!(set_current_thread_priority(ThreadPriority::Min).is_ok());
//! let result = set_current_thread_priority(ThreadPriority::Min);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I ask why did you remove the assertion? This was an example and a doc-test which was running and which was testing the library...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On macOS it blows an error because the implementation there is a stub

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the pattern which works in my application. "optional" thread priority change.

// Ignore the response- we just keep going even if priorty can not be boosted on a given platform
    let _ = thread_priority::set_current_thread_priority(thread_priority::ThreadPriority::Max);

So you can pay attention to whether it is implemented for the current OS, but as an app developer you don't have to.

@iddm
Copy link
Owner

iddm commented Mar 23, 2020 via email

@paulirotta
Copy link
Author

Excellent

@cwfitzgerald
Copy link

Any progress on this front, while not required for my use case, it would be very nice for this to also work on mac.

@iddm
Copy link
Owner

iddm commented Dec 31, 2020

Hi @cwfitzgerald,

Unfortunately, not much progress. I did some a while ago (after my last post here), but have never committed the changes and pushed. Haven't really had enough time between my projects to finally do something about it. Should you wish to work on it yourself, you are always welcome, but from my side I can't really tell when this can happen. If there were a simple and known solution to that, I'd finish it within a day, but the macOS's posix compatibility is not straightforward, and they are using cocoa to manage threads, so this required (and still requires as I have already forgotten my previous research results again) an investigation into the macOS thread management (and the best way to implement it here).

@iddm
Copy link
Owner

iddm commented Jan 22, 2022

I am trying to add the same support as we have for Linux for macOS. Check this out: #16

@iddm
Copy link
Owner

iddm commented Jan 22, 2022

Closing this in favour of #16
I also thank everyone here for your contribution! I really appreciate your feedback and effort, and I am glad you spent your time with me on making this crate a little bit better. Hope you are well!

@iddm iddm closed this Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants