-
Notifications
You must be signed in to change notification settings - Fork 16
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
Request: Remove tokio & reqwest from client FFI library #183
Comments
Why we have
With regards to What we can do:
This might cut down the size a bit. If this isn't enough, for platforms where size is a concern it may be better to write clients targeting that particular platform in the most optimal language for that platform. We can look into this as well. |
Doesn't the standard library provide an RW lock ? That way you could avoid dealing w/ any
I don't think this is accurate, from the code I can see that a runtime is being statically created inside the library itself. Also by asking to move it out, I meant that the implementation should be in the foreign language. That way you could keep the FFI library as light as possible.
Wasn't really talking about
I would disagree here. Rust's standard library provides alternatives for the things you are using
As I mentioned, to get an estimate on how much these 2 actually end up adding, I already did an experiment like this. There I am only using
This is fine, but I still think that there's value in doing a FFI library with just the business logic. That will aid in sharing the code in such cases. |
The standard library RwLock doesn't support async/await. It blocks the thread. Went with tokio since its runtime is more performant, supports async/await primitives and is more popular/well supported. Why not deal with async business? Tokio handles work stealing, thread scheduling and management.
Its not C that's supporting it, that is the underlying OS. For linux, you can use pthreads or fork() and exec(). It is different for windows. C does need OS level support to achieve this, and its different for different platforms. https://stackoverflow.com/questions/3908031/how-to-multithread-c-code
The tokio runtime that is initialized is mostly for spawning a blocking function or blocking a parallel operation to completion. If you don't run
Sure, seems I was wrong in this regard. Though replacing tokio also is a challenge:
Agreed, but in some cases the constraints may justify another approach. |
Maybe we can skip the discussions and just try to implement this? A good way to check the feasibility is to try and remove tokio and reqwest and see if we can do that without comprising perf and safety. @ShreyBana would you be willing to contribute here? |
That is a nice discussion. I think in general going granular makes sense and exposing the core CAC resolution logic as separate functions for FFI inclusion makes a lot of sense leaving the network implementation to the wrappers (while we can always supply a default network implementation wrapper) - @ShreyBana @Datron. Before we jump into implementation - let us consider the CAC lang implication. I am thinking we should finalize the CAC lang specifics within about a week and that will have client implications as well. Thanks for the detailed feedback - @ShreyBana |
Today, the FFI library is responsible for polling the server for updates. It uses
tokio
for scheduling an interval based chron &reqwest
for performing HTTP requests. The problem with this is that it'll ship these dependencies to foreign language clients adding un-necessary weight to them. To get a tangible number I created a small binary w/ these dependencies, using minimal features & configured the build for size optimizations. Seems to be a little over 1MB. You can check it out here.But size isn't the only issue, such dependencies can also limit platform support, as you can only ship to platforms supported by these libraries.
Removing
tokio
as a dependency shouldn't too hard, butreqwest
might be a challenge. Maybe one can move to a lighter HTTP client like ureq & see if the size works out, but platform support might still be a challenge.IMO, polling itself should just be moved out of the FFI library & into the foreign client.
The text was updated successfully, but these errors were encountered: