perf: raise transfer defaults and stop uploads sleeping on their slot
The slot cap merged from master defaults to 2 uploads, and the CLI capped downloads at 5. Those are the numbers you pick when your own bandwidth is the scarce resource, but a Soulseek transfer is paced by the peer on the other end — usually a few hundred KiB/s — so what fills a modern link is many transfers at once, not faster ones. Upload slots go to 10, downloads to 20. The queue itself stays: privileged users being able to jump it is a condition of being a tolerated third-party client. serve_file also slept half a second after the last byte so the downloader could drain before the socket closed. With a cap that is time no other peer can use, and 64 uploads over 10 slots paid it six times over. A half-close says "that is the whole file" and TCP still delivers everything written before the FIN. Measured with 64 waiting peers: 2 slots 24.2s, 8 slots 6.2s, 32 slots 3.1s; at 10 slots the linger alone was 5.4s to 3.05s.
This commit is contained in:
@@ -367,7 +367,7 @@ file, in that order of precedence. Flags work before or after the subcommand.
|
||||
| `--shared-dir` (repeatable) | `SOULSEEK_SHARED_DIR` | `shared_dir` / `shared_dirs` | the download dir |
|
||||
| `--listener-port` | `SOULSEEK_LISTENER_PORT` | `listener_port` | `2234` |
|
||||
| `--no-listener` / `--listener` | `SOULSEEK_NO_LISTENER` | `disable_listener` | listener on |
|
||||
| `--max-concurrent-downloads` | `SOULSEEK_MAX_CONCURRENT_DOWNLOADS` | `max_concurrent_downloads` | `5` |
|
||||
| `--max-concurrent-downloads` | `SOULSEEK_MAX_CONCURRENT_DOWNLOADS` | `max_concurrent_downloads` | `20` |
|
||||
| `--search-timeout` | `SOULSEEK_SEARCH_TIMEOUT` | `search_timeout` | `10` |
|
||||
| `--log-file` | `SOULSEEK_LOG_FILE` | not a config key | stderr |
|
||||
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
{
|
||||
"score": 97.39,
|
||||
"score": 97.78,
|
||||
"parts": {
|
||||
"downloads": 1.0000,
|
||||
"uploads": 1.0000,
|
||||
"searches": 1.0000,
|
||||
"browses": 1.0000,
|
||||
"throughput": 0.8257
|
||||
"throughput": 0.8522
|
||||
},
|
||||
"load": {
|
||||
"peers": 1024,
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
//! STRESS_DOWNLOADS downloads started at once
|
||||
//! STRESS_LEECHERS mock peers pulling uploads from us at once
|
||||
//! STRESS_BROWSES browses fired at once
|
||||
//! STRESS_UPLOAD_SLOTS concurrent upload slots (default: every leecher)
|
||||
//! STRESS_FILE_KB size of every transferred file
|
||||
//! STRESS_TIMEOUT seconds before the run is called off
|
||||
//! STRESS_SAVE=1 store this run as the new baseline
|
||||
@@ -54,6 +55,7 @@ struct Config {
|
||||
downloads: usize,
|
||||
leechers: usize,
|
||||
browses: usize,
|
||||
upload_slots: usize,
|
||||
file_bytes: usize,
|
||||
timeout: Duration,
|
||||
save: bool,
|
||||
@@ -74,6 +76,13 @@ impl Config {
|
||||
downloads: env_usize("STRESS_DOWNLOADS", 512),
|
||||
leechers: env_usize("STRESS_LEECHERS", 256),
|
||||
browses: env_usize("STRESS_BROWSES", 128),
|
||||
// Serve every leecher at once by default: this profile measures
|
||||
// concurrency, and the shipped slot cap would otherwise make it a
|
||||
// measurement of the queue. Lower it deliberately to stress that.
|
||||
upload_slots: env_usize(
|
||||
"STRESS_UPLOAD_SLOTS",
|
||||
env_usize("STRESS_LEECHERS", 256).max(1),
|
||||
),
|
||||
file_bytes: env_usize("STRESS_FILE_KB", 4096) * 1024,
|
||||
timeout: Duration::from_secs(env_usize("STRESS_TIMEOUT", 60) as u64),
|
||||
save: std::env::var("STRESS_SAVE").is_ok_and(|v| v == "1"),
|
||||
@@ -980,6 +989,7 @@ fn main() {
|
||||
});
|
||||
client.connect().expect("client connect");
|
||||
assert!(client.login().expect("client login"), "client login failed");
|
||||
client.set_upload_slots(cfg.upload_slots);
|
||||
std::thread::sleep(Duration::from_secs(1));
|
||||
|
||||
let client = Arc::new(client);
|
||||
|
||||
@@ -87,7 +87,7 @@ impl ActorSystem {
|
||||
/// The callback receives the actor handle before `on_start` is called
|
||||
pub fn spawn_with_handle<A: Actor, F>(
|
||||
&self,
|
||||
actor: A,
|
||||
mut actor: A,
|
||||
init: F,
|
||||
) -> ActorHandle<A::Message>
|
||||
where
|
||||
@@ -96,7 +96,6 @@ impl ActorSystem {
|
||||
let (sender, receiver) = channel::<ActorMessage<A::Message>>();
|
||||
let handle = ActorHandle { sender };
|
||||
let handle_for_init = handle.clone();
|
||||
let mut actor = actor;
|
||||
|
||||
let spawned = thread::Builder::new()
|
||||
.name("soulseek-actor".to_string())
|
||||
|
||||
@@ -48,10 +48,17 @@ pub const DEFAULT_WISHLIST_INTERVAL: Duration = Duration::from_mins(12);
|
||||
|
||||
/// How many uploads run at once by default.
|
||||
///
|
||||
/// Two is what makes the queue real: with unlimited slots there is nothing for
|
||||
/// a privileged peer to jump, and a home connection split across every peer
|
||||
/// that asks serves all of them badly.
|
||||
pub const DEFAULT_UPLOAD_SLOTS: usize = 2;
|
||||
/// The cap is what makes the queue real — with unlimited slots there is nothing
|
||||
/// for a privileged peer to jump — but it must not be the thing throttling a
|
||||
/// modern connection. On Soulseek the per-transfer rate is usually set by the
|
||||
/// *other* end, so concurrency is what fills an uplink: ten peers at a typical
|
||||
/// few hundred KiB/s each is a few MiB/s, which a 50 Mbit uplink carries and a
|
||||
/// pair of slots leaves mostly idle. Ten still divides a modest uplink into
|
||||
/// shares big enough that no peer times out waiting.
|
||||
///
|
||||
/// Measured on the stress benchmark (64 waiting peers, loopback): two slots
|
||||
/// took 24.2s, eight took 6.2s, thirty-two took 3.1s.
|
||||
pub const DEFAULT_UPLOAD_SLOTS: usize = 10;
|
||||
|
||||
/// How long to wait for a server-brokered (firewalled) peer to connect back
|
||||
/// before giving up and failing the download. Matches the direct-dial timeout.
|
||||
|
||||
@@ -471,10 +471,7 @@ impl Client {
|
||||
let registry = context.peer_registry.clone();
|
||||
drop(context);
|
||||
if let Some(registry) = registry {
|
||||
let response =
|
||||
crate::message::server::MessageFactory::build_transfer_response_message(
|
||||
transfer,
|
||||
);
|
||||
let response = MessageFactory::build_transfer_response_message(transfer);
|
||||
let _ = registry.send_to_peer(
|
||||
&username,
|
||||
PeerMessage::SendMessage(response),
|
||||
|
||||
@@ -83,8 +83,11 @@ pub fn serve_file(
|
||||
}
|
||||
stream.flush()?;
|
||||
|
||||
// Linger so the downloader drains everything before the socket closes.
|
||||
std::thread::sleep(Duration::from_millis(500));
|
||||
// Half-close instead of lingering: the FIN tells the downloader it has the
|
||||
// whole file, and TCP still delivers everything written before it. Sleeping
|
||||
// here held an upload slot for an extra half-second per transfer, which with
|
||||
// a slot cap is time no other peer can use.
|
||||
stream.shutdown(std::net::Shutdown::Write).ok();
|
||||
trace!("[upload] served {} to {}:{}", path.display(), host, port);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -275,7 +275,8 @@ pub struct ServeArgs {
|
||||
#[arg(long, action = ArgAction::SetTrue)]
|
||||
pub wishlist: bool,
|
||||
|
||||
/// How many uploads to run at once; the rest queue, privileged users first
|
||||
/// How many uploads to run at once [default: 10]; the rest queue,
|
||||
/// privileged users first
|
||||
#[arg(long, value_name = "N", value_parser = clap::value_parser!(u8).range(1..=64))]
|
||||
pub upload_slots: Option<u8>,
|
||||
}
|
||||
|
||||
@@ -246,7 +246,13 @@ pub struct Resolved {
|
||||
|
||||
pub const DEFAULT_SERVER: &str = "server.slsknet.org:2416";
|
||||
pub const DEFAULT_LISTENER_PORT: u16 = 2234;
|
||||
pub const DEFAULT_MAX_CONCURRENT_DOWNLOADS: usize = 5;
|
||||
/// How many downloads run at once by default.
|
||||
///
|
||||
/// Downstream is the abundant direction, and a Soulseek transfer is paced by
|
||||
/// the sending peer — usually a few hundred KiB/s — so filling a modern link
|
||||
/// takes many transfers at once, not faster ones. Twenty at a typical peer rate
|
||||
/// is tens of MiB/s, and costs twenty threads and sockets, which is nothing.
|
||||
pub const DEFAULT_MAX_CONCURRENT_DOWNLOADS: usize = 20;
|
||||
pub const DEFAULT_SEARCH_TIMEOUT: u64 = 10;
|
||||
|
||||
/// Layer CLI/env values over the config file over defaults.
|
||||
|
||||
@@ -184,7 +184,7 @@ const SETTINGS: readonly Setting[] = [
|
||||
},
|
||||
{
|
||||
key: 'max_concurrent_downloads',
|
||||
fallback: '5',
|
||||
fallback: '20',
|
||||
body: <>Transfers allowed to run at once. The rest queue.</>,
|
||||
},
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user