From 44c1a9118101e816d3a6f18b49c0f926d891a6f9 Mon Sep 17 00:00:00 2001 From: Erk Date: Thu, 29 Feb 2024 07:29:20 +0100 Subject: [PATCH] Remove Simd-json (#228) This mirrors the serenity pr https://github.com/serenity-rs/serenity/pull/2735. --- .github/workflows/ci.yml | 5 -- Cargo.toml | 1 - Makefile.toml | 11 ----- README.md | 1 - src/error.rs | 2 - src/input/adapters/cached/compressed.rs | 2 +- src/input/codecs/dca/mod.rs | 6 +-- src/input/metadata/mod.rs | 2 +- src/input/sources/ytdl.rs | 8 ++-- src/lib.rs | 4 -- src/ws.rs | 64 ++++++++++--------------- 11 files changed, 33 insertions(+), 73 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 838a92c2f..d747dc276 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,7 +35,6 @@ jobs: - Windows - driver only - gateway only - - simd json include: - name: beta toolchain: beta @@ -53,10 +52,6 @@ jobs: - name: gateway only features: gateway serenity tungstenite rustls dont-test: true - - name: simd json - features: simd-json serenity tungstenite rustls driver gateway serenity?/simd_json - rustflags: -C target-cpu=native - dont-test: true steps: - name: Checkout sources uses: actions/checkout@v3 diff --git a/Cargo.toml b/Cargo.toml index 258f427b3..9692613f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,6 @@ serde-aux = { optional = true, version = "4"} serde_json = "1" serenity = { default-features = false, optional = true, version = "0.12.0", features = ["voice", "gateway"] } serenity-voice-model = { optional = true, version = "0.2" } -simd-json = { features = ["serde_impl"], optional = true, version = "0.13" } socket2 = { optional = true, version = "0.5" } streamcatcher = { optional = true, version = "1" } symphonia = { default_features = false, optional = true, version = "0.5.2" } diff --git a/Makefile.toml b/Makefile.toml index 590aad63e..60a952779 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -8,12 +8,6 @@ args = ["fmt", "--all"] args = ["build", "--features", "full-doc"] dependencies = ["format"] -[tasks.build-simd] -args = ["build", "--features", "full-doc,simd-json,serenity?/simd_json,twilight-gateway?/simd-json"] -command = "cargo" -dependencies = ["format"] -env = { "RUSTFLAGS" = "-C target-cpu=native" } - [tasks.build-examples] args = ["build", "--manifest-path", "./examples/Cargo.toml", "--workspace"] command = "cargo" @@ -43,11 +37,6 @@ dependencies = ["format"] [tasks.test] args = ["test", "--features", "full-doc"] -[tasks.test-simd] -args = ["test", "--features", "full-doc,simd-json,serenity?/simd_json,twilight-gateway?/simd-json"] -command = "cargo" -env = { "RUSTFLAGS" = "-C target-cpu=native" } - [tasks.bench] description = "Runs performance benchmarks." category = "Test" diff --git a/README.md b/README.md index 826b587f9..4f700b3c1 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,6 @@ The library offers: a `ConnectionInfo` using any other gateway, or language for your bot, then you can run the songbird voice driver. * Voice receive and RT(C)P packet handling via the `"receive"` feature. - * SIMD-accelerated JSON decoding via the `"simd-json"` feature. * And, by default, a fully featured voice system featuring events, queues, seeking on compatible streams, shared multithreaded audio stream caches, and direct Opus data passthrough from DCA files. diff --git a/src/error.rs b/src/error.rs index 8b54ad125..a95897b97 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,8 +6,6 @@ use futures::channel::mpsc::TrySendError; pub use serde_json::Error as JsonError; #[cfg(feature = "serenity")] use serenity::gateway::ShardRunnerMessage; -#[cfg(feature = "simd-json")] -pub use simd_json::Error as JsonError; #[cfg(feature = "gateway")] use std::{error::Error, fmt}; #[cfg(feature = "twilight")] diff --git a/src/input/adapters/cached/compressed.rs b/src/input/adapters/cached/compressed.rs index 47ae98792..ad6d8531b 100644 --- a/src/input/adapters/cached/compressed.rs +++ b/src/input/adapters/cached/compressed.rs @@ -197,7 +197,7 @@ impl Compressed { )?; let mut metabytes = b"DCA1\0\0\0\0".to_vec(); let orig_len = metabytes.len(); - crate::json::to_writer(&mut metabytes, &metadata)?; + serde_json::to_writer(&mut metabytes, &metadata)?; let meta_len = (metabytes.len() - orig_len) .try_into() .map_err(|_| CodecCacheError::MetadataTooLarge)?; diff --git a/src/input/codecs/dca/mod.rs b/src/input/codecs/dca/mod.rs index 45fcf0757..826875098 100644 --- a/src/input/codecs/dca/mod.rs +++ b/src/input/codecs/dca/mod.rs @@ -110,11 +110,9 @@ impl FormatReader for DcaReader { return symph_err::decode_error("missing DCA1 metadata block"); } - let mut raw_json = source.read_boxed_slice_exact(size as usize)?; + let raw_json = source.read_boxed_slice_exact(size as usize)?; - // NOTE: must be mut for simd-json. - #[allow(clippy::unnecessary_mut_passed)] - let metadata: DcaMetadata = crate::json::from_slice::(&mut raw_json) + let metadata: DcaMetadata = serde_json::from_slice::(&raw_json) .map_err(|_| SymphError::DecodeError("malformed DCA1 metadata block"))?; let mut revision = MetadataBuilder::new(); diff --git a/src/input/metadata/mod.rs b/src/input/metadata/mod.rs index e27fd1a25..5cdce918f 100644 --- a/src/input/metadata/mod.rs +++ b/src/input/metadata/mod.rs @@ -49,7 +49,7 @@ pub struct AuxMetadata { impl AuxMetadata { /// Extract metadata and details from the output of `ffprobe -of json`. pub fn from_ffprobe_json(value: &mut [u8]) -> Result { - let output: ffprobe::Output = crate::json::from_slice(value)?; + let output: ffprobe::Output = serde_json::from_slice(value)?; Ok(output.into_aux_metadata()) } diff --git a/src/input/sources/ytdl.rs b/src/input/sources/ytdl.rs index e8f38946d..f195726f4 100644 --- a/src/input/sources/ytdl.rs +++ b/src/input/sources/ytdl.rs @@ -128,7 +128,7 @@ impl<'a> YoutubeDl<'a> { "--no-playlist", ]; - let mut output = Command::new(self.program) + let output = Command::new(self.program) .args(ytdl_args) .output() .await @@ -151,11 +151,11 @@ impl<'a> YoutubeDl<'a> { )); } - // NOTE: must be split_mut for simd-json. let out = output .stdout - .split_mut(|&b| b == b'\n') - .filter_map(|x| (!x.is_empty()).then(|| crate::json::from_slice(x))) + .split(|&b| b == b'\n') + .filter(|&x| (!x.is_empty())) + .map(|x| serde_json::from_slice(x)) .collect::, _>>() .map_err(|e| AudioStreamError::Fail(Box::new(e)))?; diff --git a/src/lib.rs b/src/lib.rs index b4b9a754b..36c006e26 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -112,11 +112,7 @@ pub use discortp as packet; #[cfg(feature = "driver")] pub use serenity_voice_model as model; -// Re-export serde-json APIs locally to minimise conditional config elsewhere. -#[cfg(not(feature = "simd-json"))] pub(crate) use serde_json as json; -#[cfg(feature = "simd-json")] -pub(crate) use simd_json::serde as json; #[cfg(feature = "driver")] pub use crate::{ diff --git a/src/ws.rs b/src/ws.rs index bda6230f2..e64e559c3 100644 --- a/src/ws.rs +++ b/src/ws.rs @@ -128,22 +128,10 @@ impl From for Error { } #[inline] -#[allow(unused_unsafe)] pub(crate) fn convert_ws_message(message: Option) -> Result> { #[cfg(feature = "tungstenite")] - return Ok(match message { - // SAFETY: - // simd-json::serde::from_str may leave an &mut str in a non-UTF state on failure. - // The below is safe as we have taken ownership of the inner `String`, and if - // failure occurs we forcibly re-validate its contents before logging. - Some(Message::Text(mut payload)) => - (unsafe { crate::json::from_str(payload.as_mut_str()) }) - .map_err(|e| { - let safe_payload = String::from_utf8_lossy(payload.as_bytes()); - debug!("Unexpected JSON: {e}. Payload: {safe_payload}"); - e - }) - .ok(), + let text = match message { + Some(Message::Text(ref payload)) => payload, Some(Message::Binary(bytes)) => { return Err(Error::UnexpectedBinaryMessage(bytes)); }, @@ -151,34 +139,32 @@ pub(crate) fn convert_ws_message(message: Option) -> Result None, - }); + _ => return Ok(None), + }; #[cfg(feature = "tws")] - return Ok(if let Some(message) = message { - if message.is_text() { - let mut payload = message.as_text().unwrap().to_owned(); - // SAFETY: - // simd-json::serde::from_str may leave an &mut str in a non-UTF state on failure. - // The below is safe as we have created an owned copy of the payload `&str`, and if - // failure occurs we forcibly re-validate its contents before logging. - (unsafe { crate::json::from_str(payload.as_mut_str()) }) - .map_err(|e| { - let safe_payload = String::from_utf8_lossy(payload.as_bytes()); - debug!("Unexpected JSON: {e}. Payload: {safe_payload}"); - e - }) - .ok() - } else if message.is_binary() { + let text = match message { + Some(ref message) if message.is_text() => + if let Some(text) = message.as_text() { + text + } else { + return Ok(None); + }, + Some(message) if message.is_binary() => { return Err(Error::UnexpectedBinaryMessage( message.into_payload().to_vec(), )); - } else if message.is_close() { + }, + Some(message) if message.is_close() => { return Err(Error::WsClosed(message.as_close().map(|(c, _)| c))); - } else { - // ping/pong; will also be internally handled by tokio-websockets - None - } - } else { - None - }); + }, + // ping/pong; will also be internally handled by tokio-websockets. + _ => return Ok(None), + }; + + Ok(serde_json::from_str(text) + .map_err(|e| { + debug!("Unexpected JSON: {e}. Payload: {text}"); + e + }) + .ok()) }