Use rustix instead of libc

[notgull]

Closes #847

rustix is a wrapper around either raw Linux system calls or libc, based
on the current platform. The main advantage is that it can make programs
much more efficient, since system calls can be inlined directly into
the functions that call them. I've seen rustix reduce instruction counts
in my programs when I've made the switch in another programs.

In addition, it reduces the amount of unsafe code.

[joshka]

Would it make sense to implement this as an alternative behind a feature flag rather than a direct replacement? Doing this would make it more convenient to compare any performance differences and behavior changes.

[notgull]

Would it make sense to implement this as an alternative behind a feature flag rather than a direct replacement? Doing this would make it more convenient to compare any performance differences and behavior changes.

I don't think so. The question comes down to how we expose this as a feature flag.

• Do we make libc the default and expose rustix as an optional feature? Then rustix users will still be lugging around a libc dependency that they might want to get rid of. And vice versa, for rustix being the default and libc being an optional feature.
• If we make them both optional and require the end user to enable them, this starts up endless bickering about feature unification and enabling them in the dependency tree. For instance, if crate a enables the libc feature and crate b enables the rustix feature, which one do you use?

It adds a lot more complexity than just using one backend does.

[joshka]

One approach to mutually exclusive feature flags like this is to use cfg(not(feature(...)) (or cfg_if). I'd imagine that we'd only need a libc feature.

My context on this is as a maintainer of Ratatui, where we generally suggest people choose Crossterm as their backend. I'd like to get a feel for whether this change has actual performance benefits, and I'm sure that being able to measure that would be nice.

Doing things with alternative implementations instead of pure replacement also carries less inherent risk - especially when there's no tests.

[notgull]

If the maintainer wants a feature flag I will add it. However I think that it will add a lot of complexity where none existing before. Ideally, such a flag would be deprecated by the v1.0 release of crossterm, after we've decided to go one way or another.

[joshka]

v1.0 release of crossterm

By definition, stable (1.x) crates can't rely on unstable (0.x) crates. Crossterm has many unstable deps, which makes this not really an impact

The main advantage is that it can make programs much more efficient, since system calls can be inlined directly into
the functions that call them.

This sounds like a fantastic benefit, that I'd like to be able to measure.

If the maintainer wants a feature flag I will add it

I want to make it clear that I'm not a crossterm maintainer, just someone that has an interest in making sure that crossterm succeeds (Ratatui apps account for ~15-25% of Crossterm's monthly download metrics).

Crossterm releases fairly slowly due to the availability of the maintainer's time. This adds weight to the idea of doing things in ways that don't require interventions down the line to address bugs or even features that work differently. The introduction of the Release and Repeat event kinds is an example of where this didn't happen, which regularly causes problems for users (so much so we leave a pinned issue at the top of the Ratatui issues page about it). Can you say with certainty that the rustix implementation (both your code, and the underlying crate) is bug free, and behavior equivalent to the existing libc code? (Maybe the answer to this is yes - like there's a test harness that proves this out in rustix or something).

I'd strongly prefer to see a change like this put at least initially behind a feature flag so that it can be tested intentionally. The default can easily be set to use rustix, but having a possibility to use libc when testing or when bugs are found is important. I'd suggest to implement this by putting the libc impl behind a flag for 0.28, and then dropping the libc implementation in 0.29 assuming all goes well.

[notgull]

I'd strongly prefer to see a change like this put at least initially behind a feature flag so that it can be tested intentionally.

Ok, I've made it so rustix is opt-in by a feature flag for now.

[joshka]

Over in #892, I redid this (taking many cues from your implementation, but instead of wrapping / changing all the libc code, I made all the changes additive in an additive fashion.

By default the rustix versions of code will be run, but adding the libc feature instead flips that to use the libc code paths.

This approach makes sure there's no modification to the libc code, and makes it super easy to remove the libc code at any point. I think this has pretty much the same effect as your code (please correct me if I've missed anything).

Does this help make a bit more sense of why I saw this as the less complex approach compared to replacing the implementation?

[notgull]

No that's actually way better than my approach. Thanks for doing that! I'll close this PR in favor of that one.