Skip to content

Commit

Permalink
Simplify flattened parsing
Browse files Browse the repository at this point in the history
Introduce separate trait for re-reading same JSON bytes again.

This is ~1.5x slower for JSON-based image array, but nowadays image arrays should be passed in optimised binary form anyway.

Meanwhile, the benefits outweight the costs - the code becomes a lot simpler, deserialization a lot less fragile, and other types of responses are actually faster to parse.
  • Loading branch information
RReverser committed Sep 15, 2024
1 parent c74bf87 commit a3ca409
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 175 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ reqwest = { version = "0.12.7", optional = true, default-features = false, featu
sailfish = { version = "0.8.3", optional = true }
serde = { version = "1.0.209", features = ["derive", "rc"] }
serde-ndim = { version = "2.0.2", optional = true, features = ["ndarray"] }
serde_json = { version = "1.0.127", features = ["raw_value"] }
serde_json = { version = "1.0.127" }
serde_plain = { version = "1.0.2", optional = true }
serde_repr = "0.1.19"
socket2 = "0.5.7"
Expand Down
2 changes: 0 additions & 2 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ pub(crate) use params::{ActionParams, Method};
mod response;
pub(crate) use response::Response;

mod parse_flattened;

use crate::api::{ConfiguredDevice, DevicePath, FallibleDeviceType, ServerInfo, TypedDevice};
use crate::response::ValueResponse;
use crate::{ASCOMError, ASCOMResult};
Expand Down
168 changes: 0 additions & 168 deletions src/client/parse_flattened.rs

This file was deleted.

34 changes: 30 additions & 4 deletions src/client/response.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,32 @@
use super::parse_flattened::Flattened;
use super::ResponseWithTransaction;
use crate::client::ResponseTransaction;
use crate::response::ValueResponse;
use crate::{ASCOMError, ASCOMErrorCode, ASCOMResult};
use bytes::Bytes;
use mime::Mime;
use serde::de::value::UnitDeserializer;
use serde::de::DeserializeOwned;
use serde::{Deserialize, Deserializer};

trait FromJsonBytes: Sized {
fn from_json_bytes(bytes: &[u8]) -> serde_json::Result<Self>;
}

impl<T: DeserializeOwned> FromJsonBytes for T {
fn from_json_bytes(bytes: &[u8]) -> serde_json::Result<Self> {
serde_json::from_slice(bytes)
}
}

#[derive(Debug)]
struct Flattened<A, B>(pub(crate) A, pub(crate) B);

impl<A: FromJsonBytes, B: FromJsonBytes> FromJsonBytes for Flattened<A, B> {
fn from_json_bytes(bytes: &[u8]) -> serde_json::Result<Self> {
Ok(Self(A::from_json_bytes(bytes)?, B::from_json_bytes(bytes)?))
}
}

pub(crate) trait Response: Sized {
fn prepare_reqwest(request: reqwest::RequestBuilder) -> reqwest::RequestBuilder {
request
Expand All @@ -18,7 +37,7 @@ pub(crate) trait Response: Sized {

struct JsonResponse<T>(T);

impl<T: DeserializeOwned> Response for JsonResponse<T> {
impl<T: FromJsonBytes> Response for JsonResponse<T> {
fn from_reqwest(mime_type: Mime, bytes: Bytes) -> eyre::Result<ResponseWithTransaction<Self>> {
eyre::ensure!(
mime_type.essence_str() == mime::APPLICATION_JSON.as_ref(),
Expand All @@ -31,7 +50,7 @@ impl<T: DeserializeOwned> Response for JsonResponse<T> {
};

let Flattened(transaction, response) =
serde_json::from_slice::<Flattened<ResponseTransaction, T>>(&bytes)?;
<Flattened<ResponseTransaction, T>>::from_json_bytes(&bytes)?;

Ok(ResponseWithTransaction {
transaction,
Expand All @@ -47,7 +66,14 @@ impl<T: DeserializeOwned> Response for ASCOMResult<T> {
impl<'de, T: Deserialize<'de>> Deserialize<'de> for ParseResult<T> {
fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
Ok(Self(
T::deserialize(deserializer).map_err(|err| eyre::eyre!("{err}")),
if size_of::<T>() == 0 {
// serde doesn't consider empty maps to be a valid source for the `()` type.
// We could tweak the type itself, but it's easier to just special-case empty types here.
T::deserialize(UnitDeserializer::new())
} else {
T::deserialize(deserializer)
}
.map_err(|err| eyre::eyre!("{err}")),
))
}
}
Expand Down

0 comments on commit a3ca409

Please sign in to comment.