lazily create a static tokio runtime object that can be passed around for client calls. create the ffi module where library APIs will go.
Details
Details
- Reviewers
tomek • karol - Commits
- rCOMM00e7acfc2606: [native] create bridge module and runtime
cargo build
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- identity-client (branched from master)
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Seems like creating runtime and including FFI aren't related tasks - maybe they should be separated into two diffs?
| native/cpp/CommonCpp/grpc/grpc_client/src/lib.rs | ||
|---|---|---|
| 11–12 | What is the reason behind setting these values? worker_threads defaults to the number of cores - is there something wrong with this value? | |
Comment Actions
remove bridge module
| native/cpp/CommonCpp/grpc/grpc_client/src/lib.rs | ||
|---|---|---|
| 11–12 | Since this library is for the native app, I think we should first make sure the runtime works on a single thread, and then we can look into using more | |
| native/cpp/CommonCpp/grpc/grpc_client/src/lib.rs | ||
|---|---|---|
| 11–12 | Ah, ok, that makes sense. But at the same time, it might be a good idea to perform some testing to check if we can use more threads in the future. If it happens that iOS limits us significantly, the sooner we know about it the better. | |
| native/cpp/CommonCpp/grpc/grpc_client/src/lib.rs | ||
|---|---|---|
| 11–12 | One additional thing to consider is that we were trying to be able to respond to iOS notifications without using additional threads (before the app is started). The reason was that we believed that using more threads could make the notifications less likely to be delivered. If using gRPC always happens using separate thread, it could be a potential issue for us. CC @ashoat | |
| native/cpp/CommonCpp/grpc/grpc_client/src/lib.rs | ||
|---|---|---|
| 11–12 | Agree we should use as few threads as possible. Not sure how easy it would be to have the main thread handle gRPC... assume it would require blocking calls, and would preclude the use of a separate runtime... so would probably have to be a custom codepath. With that said, I do think we should make sure to keep the thread pool minimal in the background notif case as well as the Notification Service Extension case. Before we increase this number beyond 1, we should do some work to make it possible to detect the context, so we can keep it to 1 for the above cases. | |