Skip to content
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

WorkerSettings: Add disableLiburing option (enable_liburing in Rust) #1442

Merged
merged 15 commits into from
Aug 12, 2024
2 changes: 1 addition & 1 deletion .github/workflows/mediasoup-rust.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:
run: cargo clippy --all-targets -- -D warnings

# NOTE: In Windows this will build and test libmediasoupworker in release
# mode twice since build.rs doesn't allow debug mode for Windows.
# mode twice since build.rs doesn't allow debug mode on Windows.
- name: cargo test
run: |
cargo test --verbose
Expand Down
23 changes: 12 additions & 11 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- CI: Support Node 22 ([PR #1434](https://github.com/versatica/mediasoup/pull/1434)).
- Update ESLint to version 9 ([PR #1435](https://github.com/versatica/mediasoup/pull/1435)).
- `Worker`: Add `disableLiburing` boolean option (`false` by default) to disable `io_uring` even if it's supported by the prebuilt `mediasoup-worker` and by current host ([PR #1442](https://github.com/versatica/mediasoup/pull/1442)).

### 3.14.9

Expand Down Expand Up @@ -246,11 +247,11 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi

### 3.12.2

- CI: Use `ubuntu-20.04` to build mediasoup-worker prebuilt on Linux ([PR #1092](https://github.com/versatica/mediasoup/pull/1092)).
- CI: Use `ubuntu-20.04` to build `mediasoup-worker` prebuilt on Linux ([PR #1092](https://github.com/versatica/mediasoup/pull/1092)).

### 3.12.1

- mediasoup-worker prebuild: Fallback to local building if fetched binary doesn't run on current host ([PR #1090](https://github.com/versatica/mediasoup/pull/1090)).
- `mediasoup-worker` prebuild: Fallback to local building if fetched binary doesn't run on current host ([PR #1090](https://github.com/versatica/mediasoup/pull/1090)).

### 3.12.0

Expand Down Expand Up @@ -664,7 +665,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi
- `Transport`: Implement new `setMaxOutgoingBitrate()` method ([PR #555](https://github.com/versatica/mediasoup/pull/555) by @t-mullen).
- `SctpAssociation`: Don't warn if SCTP send buffer is full.
- Rust: Update modules structure and other minor improvements for Rust version ([PR #558](https://github.com/versatica/mediasoup/pull/558)).
- `mediasoup-worker`: Avoid duplicated basenames so that libmediasoup-worker is compilable on macOS ([PR #557](https://github.com/versatica/mediasoup/pull/557)).
- `mediasoup-worker`: Avoid duplicated basenames so that `libmediasoup-worker` is compilable on macOS ([PR #557](https://github.com/versatica/mediasoup/pull/557)).

### 3.7.5

Expand Down Expand Up @@ -777,8 +778,8 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi

### 3.6.20

- Remove `-fwrapv` when building mediasoup-worker in `Debug` mode (issue #460).
- Add `MEDIASOUP_MAX_CORES` to limit `NUM_CORES` during mediasoup-worker build ([PR #462](https://github.com/versatica/mediasoup/pull/462)).
- Remove `-fwrapv` when building `mediasoup-worker` in `Debug` mode (issue #460).
- Add `MEDIASOUP_MAX_CORES` to limit `NUM_CORES` during `mediasoup-worker` build ([PR #462](https://github.com/versatica/mediasoup/pull/462)).

### 3.6.19

Expand Down Expand Up @@ -894,7 +895,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi

- SCTP/DataChannel termination:
- [PR #409](https://github.com/versatica/mediasoup/pull/409)
- Allow the Node application to directly send text/binary messages to mediasoup-worker C++ process so others can consume them using `DataConsumers`.
- Allow the Node application to directly send text/binary messages to `mediasoup-worker` C++ process so others can consume them using `DataConsumers`.
- And vice-versa: allow the Node application to directly consume in Node messages send by `DataProducers`.
- Add `WorkerLogTag` TypeScript enum and also add a new 'message' tag into it.

Expand Down Expand Up @@ -944,7 +945,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi

### 3.5.7

- Fix crash in mediasoup-worker due to conversion from `uint64_t` to `int64_t` (used within `libwebrtc` code. Fixes #357.
- Fix crash in `mediasoup-worker` due to conversion from `uint64_t` to `int64_t` (used within `libwebrtc` code. Fixes #357.
- Update `usrsctp` library.
- Update Node deps.

Expand Down Expand Up @@ -1058,7 +1059,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi

### 3.4.1

- Improve mediasoup-worker build system by using `sh` instead of `bash` and default to 4 cores (thanks @smoke, [PR #349](https://github.com/versatica/mediasoup/pull/349)).
- Improve `mediasoup-worker` build system by using `sh` instead of `bash` and default to 4 cores (thanks @smoke, [PR #349](https://github.com/versatica/mediasoup/pull/349)).

### 3.4.0

Expand Down Expand Up @@ -1133,7 +1134,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi
### 3.2.1

- Add RTCP Extended Reports for RTT calculation on receiver RTP stream (thanks @yangjinechofor for initial pull request #314).
- Make mediasoup-worker compile in Armbian Debian Buster (thanks @krishisola, fixes #321).
- Make `mediasoup-worker` compile in Armbian Debian Buster (thanks @krishisola, fixes #321).

### 3.2.0

Expand Down Expand Up @@ -1255,7 +1256,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi

### 2.6.13

- Make mediasoup-worker compile in Armbian Debian Buster (thanks @krishisola, fixes #321).
- Make `mediasoup-worker` compile in Armbian Debian Buster (thanks @krishisola, fixes #321).
- Update deps.

### 2.6.12
Expand All @@ -1281,7 +1282,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi

### 2.6.7

- Fix wrong destruction of Transports in Router.cpp that generates 100% CPU usage in mediasoup-worker processes.
- Fix wrong destruction of Transports in Router.cpp that generates 100% CPU usage in `mediasoup-worker` processes.

### 2.6.6

Expand Down
10 changes: 10 additions & 0 deletions node/src/Worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ export type WorkerSettings<WorkerAppData extends AppData = AppData> = {
*/
libwebrtcFieldTrials?: string;

/**
* Disable liburing (io_uring) despite it's supported in current host.
*/
disableLiburing?: boolean;

/**
* Custom application data.
*/
Expand Down Expand Up @@ -287,6 +292,7 @@ export class Worker<
dtlsCertificateFile,
dtlsPrivateKeyFile,
libwebrtcFieldTrials,
disableLiburing,
appData,
}: WorkerSettings<WorkerAppData>) {
super();
Expand Down Expand Up @@ -338,6 +344,10 @@ export class Worker<
spawnArgs.push(`--libwebrtcFieldTrials=${libwebrtcFieldTrials}`);
}

if (disableLiburing) {
spawnArgs.push(`--disableLiburing=true`);
}

logger.debug(
'spawning worker process: %s %s',
spawnBin,
Expand Down
1 change: 1 addition & 0 deletions node/src/test/test-Worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ test('createWorker() succeeds', async () => {
dtlsCertificateFile: path.join(__dirname, 'data', 'dtls-cert.pem'),
dtlsPrivateKeyFile: path.join(__dirname, 'data', 'dtls-key.pem'),
libwebrtcFieldTrials: 'WebRTC-Bwe-AlrLimitedBackoff/Disabled/',
disableLiburing: true,
appData: { foo: 456 },
});

Expand Down
2 changes: 1 addition & 1 deletion node/src/test/test-node-sctp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ beforeEach(async () => {
// Set node-sctp default PMTU to 1200.
sctp.defaults({ PMTU: 1200 });

ctx.worker = await mediasoup.createWorker();
ctx.worker = await mediasoup.createWorker({ disableLiburing: true });
ctx.router = await ctx.worker.createRouter();
ctx.plainTransport = await ctx.router.createPlainTransport({
// https://github.com/nodejs/node/issues/14900.
Expand Down
1 change: 1 addition & 0 deletions rust/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# NEXT

- Update Rust toolchain channel to version 1.79.0 (PR #1409).
- Updates from mediasoup TypeScript `3.14.7..=3.14.10`.

# 0.17.0

Expand Down
5 changes: 4 additions & 1 deletion rust/src/router/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ async fn init() -> Worker {
let worker_manager = WorkerManager::new();

worker_manager
.create_worker(WorkerSettings::default())
.create_worker(WorkerSettings {
enable_liburing: false,
..WorkerSettings::default()
})
Comment on lines +19 to +22
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nazar-pc, despite this works at intended (default settings are used but enable_liburing which is set to false instead), is this correct idiomatic syntax in Rust?

In Node the default object must be added first, then those fields whose value must be different:

options = { ...options, foo: false }

It surprises me that in Rust it works if the changes are added first before the object with default values.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, I believe .. should be used at the very end, I don't think it compiles otherwise at all

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. For me idiomatic would be "here some values and after them those that override them" but it's ok. Rust guys have their own anti idiomatic concept of what idiomatic should mean.

.await
.expect("Failed to create worker")
}
Expand Down
13 changes: 13 additions & 0 deletions rust/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ pub struct WorkerSettings {
/// "WebRTC-Bwe-AlrLimitedBackoff/Enabled/".
#[doc(hidden)]
pub libwebrtc_field_trials: Option<String>,
/// Enable liburing This option is ignored if io_uring is not supported by
/// current host.
///
/// Default `true`.
pub enable_liburing: bool,
/// Function that will be called under worker thread before worker starts, can be used for
/// pinning worker threads to CPU cores.
pub thread_initializer: Option<Arc<dyn Fn() + Send + Sync>>,
Expand Down Expand Up @@ -221,6 +226,7 @@ impl Default for WorkerSettings {
rtc_port_range: 10000..=59999,
dtls_files: None,
libwebrtc_field_trials: None,
enable_liburing: true,
thread_initializer: None,
app_data: AppData::default(),
}
Expand All @@ -235,6 +241,7 @@ impl fmt::Debug for WorkerSettings {
rtc_port_range,
dtls_files,
libwebrtc_field_trials,
enable_liburing,
thread_initializer,
app_data,
} = self;
Expand All @@ -245,6 +252,7 @@ impl fmt::Debug for WorkerSettings {
.field("rtc_port_range", &rtc_port_range)
.field("dtls_files", &dtls_files)
.field("libwebrtc_field_trials", &libwebrtc_field_trials)
.field("enable_liburing", &enable_liburing)
.field(
"thread_initializer",
&thread_initializer.as_ref().map(|_| "ThreadInitializer"),
Expand Down Expand Up @@ -356,6 +364,7 @@ impl Inner {
rtc_port_range,
dtls_files,
libwebrtc_field_trials,
enable_liburing,
thread_initializer,
app_data,
}: WorkerSettings,
Expand Down Expand Up @@ -404,6 +413,10 @@ impl Inner {
));
}

if !enable_liburing {
spawn_args.push("--disableLiburing=true".to_string());
}

let id = WorkerId::new();
debug!(
"spawning worker with arguments [id:{}]: {}",
Expand Down
2 changes: 1 addition & 1 deletion worker/include/DepLibUring.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class DepLibUring
class LibUring;

// Whether liburing is enabled or not after runtime checks.
static bool enabled;
thread_local static bool enabled;
thread_local static LibUring* liburing;

public:
Expand Down
1 change: 1 addition & 0 deletions worker/include/Settings.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class Settings
std::string dtlsCertificateFile;
std::string dtlsPrivateKeyFile;
std::string libwebrtcFieldTrials{ "WebRTC-Bwe-AlrLimitedBackoff/Enabled/" };
bool liburingDisabled{ false };
};

public:
Expand Down
14 changes: 11 additions & 3 deletions worker/src/DepLibUring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@
#include "DepLibUring.hpp"
#include "Logger.hpp"
#include "MediaSoupErrors.hpp"
#include "Settings.hpp"
#include "Utils.hpp"
#include <sys/eventfd.h>
#include <sys/resource.h>
#include <sys/utsname.h>

/* Static variables. */
bool DepLibUring::enabled{ false };
/* liburing instance per thread. */
thread_local bool DepLibUring::enabled{ false };
// liburing instance per thread.
thread_local DepLibUring::LibUring* DepLibUring::liburing{ nullptr };
/* Completion queue entry array used to retrieve processes tasks. */
// Completion queue entry array used to retrieve processes tasks.
thread_local struct io_uring_cqe* cqes[DepLibUring::QueueDepth];

/* Static methods for UV callbacks. */
Expand Down Expand Up @@ -121,6 +122,13 @@ void DepLibUring::ClassInit()

MS_DEBUG_TAG(info, "liburing version: \"%i.%i\"", mayor, minor);

if (Settings::configuration.liburingDisabled)
{
MS_DEBUG_TAG(info, "liburing disabled by user settings");

return;
}

// This must be called first.
DepLibUring::CheckRuntimeSupport();

Expand Down
18 changes: 16 additions & 2 deletions worker/src/Settings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ void Settings::SetConfiguration(int argc, char* argv[])
{ "dtlsCertificateFile", optional_argument, nullptr, 'c' },
{ "dtlsPrivateKeyFile", optional_argument, nullptr, 'p' },
{ "libwebrtcFieldTrials", optional_argument, nullptr, 'W' },
{ nullptr, 0, nullptr, 0 }
{ "disableLiburing", optional_argument, nullptr, 'd' },
{ nullptr, 0, nullptr, 0 }
};
// clang-format on
std::string stringValue;
Expand All @@ -73,11 +74,12 @@ void Settings::SetConfiguration(int argc, char* argv[])

optind = 1; // Set explicitly, otherwise subsequent runs will fail.
opterr = 0; // Don't allow getopt to print error messages.

while ((c = getopt_long_only(argc, argv, "", options, &optionIdx)) != -1)
{
if (!optarg)
{
MS_THROW_TYPE_ERROR("unknown configuration parameter: %s", optarg);
MS_THROW_TYPE_ERROR("missing value in command line argument in option '%c'", c);
}

switch (c)
Expand Down Expand Up @@ -158,6 +160,18 @@ void Settings::SetConfiguration(int argc, char* argv[])
break;
}

case 'd':
{
stringValue = std::string(optarg);

if (stringValue == "true")
{
Settings::configuration.liburingDisabled = true;
}

break;
}

// Invalid option.
case '?':
{
Expand Down
Loading