rust3ds / ctru-rs

Rust wrapper for libctru
https://rust3ds.github.io/ctru-rs/
Other
116 stars 17 forks source link

Wiki: mention `std::thread::yield_now` as an option for yielding from threads #183

Closed CenTdemeern1 closed 2 months ago

CenTdemeern1 commented 3 months ago

Relevant page: System Flaws

In some situations, using yield_now may be preferred over sleep, which is not mentioned in the relevant paragraph. That's a shame, because it can be really really useful if you know about it, especially because co-operative threading seems to be its intended purpose. Instead, the paragraph makes it sound like sleep is the only option for something like this, which stopped me from researching this further until today.

Suggested edit:

For cooperative threads using std, this is done via std::thread::sleep or via internal waits in functions like Mutex::lock.

Becomes:

For cooperative threads using std, this is done via std::thread::sleep, std::thread::yield_now, or via internal waits in functions like Mutex::lock.

Why is this an issue and not a pull request?

Unfortunately, GitHub doesn't support making pull requests on Wikis. If I could make a pull request, make an edit suggestion some other way, or edit the wiki directly, I would've preferred that, but alas.

Meziu commented 3 months ago

That's true, and in the first revision of that Wiki I wanted to add that in as an option, since it always seemed to be glossed over when talking about threading. However, there are some reasons why I decided against adding it.

The default implementation within std for std::thread::yield_now() uses sched_yield here. sched_yield is the pthread function to handle this sort of thing. The problem is pretty clear when looking at the implementation devkitARM uses for this function: sched_yield (void).

...nothing. It's unimplemented. Which was honestly pretty weird when I looked at it back in the day.

During my research, I came to the understanding that there was no mention of a way for threads to be yielded at will in Horizon OS. As explained here, the threads with the highest priority will always run to completion first when available, which means that yielding has no effect if no "lower priority" thread exists. It also has no effect if an "higher priority" thread is available, since that would be run regardless. That effectively means that the only case in which such an action would be useful is when multiple threads with the same priority are running concurrently, for which you may want to switch from one to the other. That's probably not the most important feature for such a scheduler (or for a user which is programming for the platform) since most of the concurrent code should be written by keeping in mind the associated sched priority, and not with the hope that yielding the thread will run what you actually want.

3dbrew does mention a "dynamic priority" value (which is usually the "queue" index assigned to the thread in SCHED_FIFO) but it's unclear whether it's possible to modify it during runtime.

Either way, back then I wasn't deterred from trying to make yield work, so I implemented sched_yield in pthread_3ds myself here. This uses svcThreadSleep as an hopeful replacement, since it has actually never been tested. As the docs for that function say, the number of milliseconds specified is the "minimum" amount of time spent waiting, so I'm unsure whether the scheduler even takes into consideration 0 as a parameter (0 was chosen as that's a working replacement for many other operating systems).

If during testing you come to the realization that yield_now is actually working (or could work by changing the code in pthread-3ds) I will consider adding it in the wiki :+1:.

CenTdemeern1 commented 3 months ago

If during testing you come to the realization that yield_now is actually working (or could work by changing the code in pthread-3ds) I will consider adding it in the wiki 👍.

Before making this issue I did actually test yield_now to make sure it actually worked. From my testing, it did! I'll quickly push my testing code into a branch (I used an existing test app I made for this) so you can test it and see it in action yourself.

CenTdemeern1 commented 3 months ago

Update: After doing some more testing, it does indeed work, but it's not entirely stable (crashes when you return from the home menu?) Here is that branch I mentioned, be sure to read the code comments I added: https://github.com/CenTdemeern1/aris-3ds/tree/yield_now

Meziu commented 2 months ago

Update: After doing some more testing, it does indeed work, but it's not entirely stable (crashes when you return from the home menu?)

Without considering the crashes (and the fact that what you showed isn't really a minimal test 😅), there might be some issues with the use of gfx.wait_for_vblank, since that would give time to the audio code to run either way. I need something a bit more concise than that, that doesn't really get into other thread-related code.

CenTdemeern1 commented 2 months ago

and the fact that what you showed isn't really a minimal test 😅

I don't exactly recall any mention of me needing to make a minimal example/test/repro, I just worked with what I had

there might be some issues with the use of gfx.wait_for_vblank, since that would give time to the audio code to run either way. I need something a bit more concise than that, that doesn't really get into other thread-related code.

Got it. I'll see what I can do when I have the time. The main problem with making a minimal example for this kind of thing is proving that it's actually working (which I could do more easily with the audio code I knew would have very clearly visible issues if it didn't work) so I have to think up a solution for that.

Meziu commented 2 months ago

Actually, it seems to work in a very pretty way. Here's my example:

use ctru::prelude::*;

fn main() {
    let gfx = Gfx::new().expect("Couldn't obtain GFX controller");
    let mut hid = Hid::new().expect("Couldn't obtain HID controller");
    let apt = Apt::new().expect("Couldn't obtain APT controller");
    let _console = Console::new(gfx.top_screen.borrow_mut());

    std::thread::spawn(|| {
        println!("I'm in the new thread!");
        loop {
            // Comment out this line too see the change in behaviour.
            std::thread::yield_now();
        }
    });

    // We sleep the main thread for a moment since (for SCHED_FIFO) it would run before the new thread otherwise.
    std::thread::sleep(std::time::Duration::from_nanos(1));

    // Without yielding, this point will never be reached.
    println!("I'm in the main thread!");

    println!("\x1b[29;16HPress Start to exit");

    while apt.main_loop() {
        hid.scan_input();

        if hid.keys_down().contains(KeyPad::START) {
            break;
        }

        gfx.wait_for_vblank();
    }
}

I'll add it to the wiki, I'm convinced now :+1: