Move the Reactor to its own module

[duarten]

What does this PR do?

This will make it easier to introduce multiple Reactors, such as a no-op
one (to use glommio for its task scheduling, TPC capabilities) or one
suited for a different platform, like macOS.

Motivation

The larger goal is to make it smooth to develop for glommio in macOS.

Related issues

Additional Notes

Checklist

[] I have added unit tests to the code I am submitting
[] My unit tests cover both failure and success scenarios
[] If applicable, I have discussed my architecture

[glommer]

I am fine with this direction, but I was hoping more code would be shared in parking.rs than currently. It seems like you will eventually have to implement all of the operation functions (like read and write), to all reactors, but ultimately all they are doing is creating a Source and passing it down to the per-system reactor.

Why did you feel like this level of abstraction is the best ?

[duarten]

I am fine with this direction, but I was hoping more code would be shared in parking.rs than currently.

I'm not sure we disagree on the direction, I think I didn't properly explain my thinking. I didn't mean to say that there won't be any code sharing. parking.rs has two aspects to it, actual parking and the reactor. If we add more code to the reactor aspect, then I think it makes sense to move it to a separate file.

A way to share code is to make Reactor generic on Flavor, have an implementation for all flavors (impl<T: Flavour> for Reactor<T>) and specific impls for each specific flavor, as needed.

It seems like you will eventually have to implement all of the operation functions (like read and write), to all reactors, but ultimately all they are doing is creating a Source and passing it down to the per-system reactor.

Why did you feel like this level of abstraction is the best ?

Per-system code will still live in sys, but for some reactors we may want to pick and choose which operations they expose. For a particular flavor, we may not expose any tcp functions. Having that behind an impl allows me to apply a conditional compilation attribute to the whole impl, instead of to individual functions.

Relying on conditional compilation attributes avoids runtime panics, but maybe that's too much work; in that case it makes total sense to change the type of Reactor::sys and panic!() some functions.

[glommer]

makes sense.

[glommer]

FWIW: CI is not running for you. @HippoBaro recently found out that this is because you don't have a circleCI account. Likely if we don't find a way around this, ideally we'd move away from circleCI.

[duarten]

I'll create an account! I can volunteer some time to move to github actions if you want to get rid of this hurdle.

[HippoBaro]

I'll create an account! I can volunteer some time to move to github actions if you want to get rid of this hurdle.

If you can spare the time, that would be great! I did the initial Circle CI setup but I have to admit that I'm not very knowledgeable when it comes to that stuff.

Are you suggesting that we could use GH actions to trigger CI for PRs created by external contributors?

To be honest I'm still in disbelief that Circle CI can't manage to do that by default.

[duarten]

I'll create an account! I can volunteer some time to move to github actions if you want to get rid of this hurdle.

If you can spare the time, that would be great! I did the initial Circle CI setup but I have to admit that I'm not very knowledgeable when it comes to that stuff.

Are you suggesting that we could use GH actions to trigger CI for PRs created by external contributors?

To be honest I'm still in disbelief that Circle CI can't manage to do that by default.

Github actions would be the CI. Basically move .circleci/config.yml to .github/workflows/{meaningful name}.yml and translate the yaml. Github actions even includes rusts and cargo, with clippy and rustfmt installed.

[HippoBaro]

I'll create an account! I can volunteer some time to move to github actions if you want to get rid of this hurdle.

If you can spare the time, that would be great! I did the initial Circle CI setup but I have to admit that I'm not very knowledgeable when it comes to that stuff.

Are you suggesting that we could use GH actions to trigger CI for PRs created by external contributors?

To be honest I'm still in disbelief that Circle CI can't manage to do that by default.

Github actions would be the CI. Basically move .circleci/config.yml to .github/workflows/{meaningful name}.yml and translate the yaml. Github actions even includes rusts and cargo, with clippy and rustfmt installed.

Ok that makes sense. However we may want to hold off on that one. The reason being that we currently are unable to run the unit tests in our current CI because the kernel isn't new enough.

If GH actions have a good kernel then great, but otherwise it may be worth investigating a longer term solution. Wdyt?

[duarten]

If GH actions have a good kernel then great, but otherwise it may be worth investigating a longer term solution. Wdyt?

The kernel version is 5.8.0-1041-azure. I think that's enough?

But if we outpace the kernel of the GH hosted runners, we have the option of providing self-hosted runners.

[glommer]

5.8 is good enough

[HippoBaro]

LGTM!