Implement backend-webrtc-rs so we can compare performance with backend-go-pion #39

Connoropolous · 2023-05-28T18:27:54Z

Trying to use backend-webrtc-rs instead of backend-go-pion but it won't compile.

This is running cargo build --release --target aarch64-apple-ios, but maybe it also happens for normal targets. Not sure yet, haven't checked.

error[E0433]: failed to resolve: use of undeclared crate or module `imp`
  --> /Users/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tx5-0.0.1-alpha.17/src/back_buf.rs:13:10
   |
13 |     imp: imp::ImpWriter,
   |          ^^^ use of undeclared crate or module `imp`

error[E0433]: failed to resolve: use of undeclared crate or module `imp`
  --> /Users/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tx5-0.0.1-alpha.17/src/back_buf.rs:22:18
   |
22 |             imp: imp::ImpWriter::new()?,
   |                  ^^^ use of undeclared crate or module `imp`

error[E0433]: failed to resolve: use of undeclared crate or module `imp`
  --> /Users/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tx5-0.0.1-alpha.17/src/back_buf.rs:65:21
   |
65 |     pub(crate) imp: imp::Imp,
   |                     ^^^ use of undeclared crate or module `imp`

error[E0433]: failed to resolve: use of undeclared crate or module `tx5_go_pion`
  --> /Users/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tx5-0.0.1-alpha.17/src/back_buf.rs:76:33
   |
76 |     pub(crate) fn from_raw(buf: tx5_go_pion::GoBuf) -> Self {
   |                                 ^^^^^^^^^^^ use of undeclared crate or module `tx5_go_pion`

error[E0433]: failed to resolve: use of undeclared crate or module `imp`
  --> /Users/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tx5-0.0.1-alpha.17/src/back_buf.rs:78:18
   |
78 |             imp: imp::Imp::from_raw(buf),
   |                  ^^^ use of undeclared crate or module `imp`

error[E0433]: failed to resolve: use of undeclared crate or module `imp`
  --> /Users/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tx5-0.0.1-alpha.17/src/back_buf.rs:87:18
   |
87 |             imp: imp::Imp::from_slice(slice)?,
   |                  ^^^ use of undeclared crate or module `imp`

error[E0433]: failed to resolve: use of undeclared crate or module `imp`
   --> /Users/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tx5-0.0.1-alpha.17/src/back_buf.rs:102:18
    |
102 |             imp: imp::Imp::from_json(s)?,
    |                  ^^^ use of undeclared crate or module `imp`

error[E0425]: cannot find value `on_new_conn` in module `endpoint`
   --> /Users/connor/.cargo/registry/src/github.com-1ecc6299db9ec823/tx5-0.0.1-alpha.17/src/config.rs:320:55
    |
320 |                 .unwrap_or_else(|| Arc::new(endpoint::on_new_conn));
    |                                                       ^^^^^^^^^^^ not found in `endpoint`

The text was updated successfully, but these errors were encountered:

neonphog · 2023-05-29T15:35:38Z

@Connoropolous - webrtc-rs isn't implemented at all, I just wanted to make sure the feature switching logic was in place to ensure the availability of that path when it's ready.

There are some issues I'm keeping my eye on:

In general the lib doesn't feel ready for prime-time. There are anecdotally many performance issues with it, and lots of churn / immature aspects that just need some time to sort out.

That being said, if someone had time, we do want to see it implemented so we can start to get some apples-to-apples comparisons and be able to make an informed decision about when we/it are ready to switch to as a default.

Connoropolous · 2023-05-29T17:13:03Z

I see. Ok, thanks.
Do you have any immediate ideas or perspective on the apparent tx5 panic mentioned here?
https://hackmd.io/_CFnGaWvQg6Ys3DxZblmug#May-28
It seems the next step is to track it down

Connoropolous · 2023-05-29T17:24:23Z

It seems likely to stem from an unwrap as there's no error specific error message, true?
https://github.com/search?q=repo%3Aholochain%2Ftx5%20.unwrap&type=code

neonphog · 2023-05-29T17:37:54Z

@Connoropolous - oo, that actually looks like a panic in the GO code... unfortunately both languages use that term.

I have a highest level panic catch in the go lib, but for this case I think I might need to add a lower-level one so the error can be returned with the more relevant call. Well... I'll look into it in any case, and create a new issue for this.

Thanks for finding all these issues!!

Connoropolous · 2023-05-29T18:05:43Z

Ok fantastic.

Connoropolous · 2023-05-29T18:08:05Z

if you need anything to construct steps to reproduce, let me know

neonphog · 2023-05-29T19:12:57Z

#40

neonphog · 2023-05-29T23:31:04Z

@Connoropolous - If it's okay, I'll leave this open for actually implementing the webrtc-rs backend. I changed the title : )

Connoropolous · 2023-05-30T00:25:16Z

Ok cool

Connoropolous · 2023-05-31T02:45:56Z

@neonphog I have no idea whether we'd be up for the challenge of this at all, without a sense of the scope... can you give me an impression of the work involved in doing this?

Connoropolous · 2023-05-31T02:51:10Z

is it kind of like implementing the equivalent of the things that are marked with the backend-go-pion feature?

https://github.com/search?q=repo%3Aholochain%2Ftx5%20backend-go-pion&type=code

neonphog · 2023-05-31T17:15:13Z

@Connoropolous - yep exactly, the good part is that the go version is already implemented so you can use that as a guide. Some challenges of unknown size:

The go lib requires a very odd go buffer type. That has been abstracted too, but I don't know how easy it will be to make the webrtc-rs buffer type (Vec<u8>? bytes::Buf?) work with it.
There are undoubtedly some points in the codebase where the abstraction has leaked (because we only had the one backend type) that will need to be cleaned up.
It's possible the unit tests will run properly with cargo test --no-default-features --features backend-webrtc-rs... but if not, that'd need to be fixed, and it'd be good to have the CI system updated to run the tests on both features.

Connoropolous · 2023-06-01T05:26:06Z

just cause I was curious, I tried something, can you check if its in the right direction.
#41

github-actions · 2023-09-26T21:02:36Z