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

Add faster serde serialization, sacrificing invariant-safety for speed in release mode; test that it roudtrips #252

Closed
wants to merge 4 commits into from
Next

Add faster serde serialization, sacrificing invariant-safety for spee…

…d in release mode; test that it roudtrips
  • Loading branch information
Manishearth committed Dec 17, 2016
commit ad8d172b7e220a752003d259d507ba7eb9d9d23d
@@ -24,6 +24,8 @@ test = false
[dev-dependencies]
rustc-test = "0.1"
rustc-serialize = "0.3"
serde = "0.8.20"
bincode = {version = "0.6.0", features = ["serde"] }

[features]
query_encoding = ["encoding"]
@@ -35,4 +37,4 @@ heapsize = {version = ">=0.1.1, <0.4", optional = true}
idna = { version = "0.1.0", path = "./idna" }
matches = "0.1"
rustc-serialize = {version = "0.3", optional = true}
serde = {version = ">=0.6.1, <0.9", optional = true}
serde = {version = "0.8.20", optional = true}
@@ -16,6 +16,9 @@ use parser::{ParseResult, ParseError};
use percent_encoding::percent_decode;
use idna;

#[cfg(feature="serde")]
use serde;

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum HostInternal {
None,
@@ -27,6 +30,39 @@ pub enum HostInternal {
#[cfg(feature = "heapsize")]
known_heap_size!(0, HostInternal);

#[cfg(feature="serde")]
impl serde::Serialize for HostInternal {
fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error> where S: serde::Serializer {
match *self {
HostInternal::None => 0u8.serialize(serializer),
HostInternal::Domain => 1u8.serialize(serializer),
HostInternal::Ipv4(addr) => {
2u8.serialize(serializer)?;

This comment has been minimized.

@erickt

erickt Dec 17, 2016

would be better as (2u8, addr).serialize(serializer)

addr.serialize(serializer)
}
HostInternal::Ipv6(addr) => {
3u8.serialize(serializer)?;

This comment has been minimized.

addr.serialize(serializer)
}
}
}
}

#[cfg(feature="serde")]
impl serde::Deserialize for HostInternal {
fn deserialize<D>(deserializer: &mut D) -> Result<Self, D::Error> where D: serde::Deserializer {
use serde::{Deserialize, Error};
let discr: u8 = Deserialize::deserialize(deserializer)?;
match discr {
0 => Ok(HostInternal::None),
1 => Ok(HostInternal::Domain),
2 => Ok(HostInternal::Ipv4(Deserialize::deserialize(deserializer)?)),
3 => Ok(HostInternal::Ipv6(Deserialize::deserialize(deserializer)?)),
_ => Err(D::Error::unknown_variant("Unknown variant")),
}
}
}

impl<S> From<Host<S>> for HostInternal {
fn from(host: Host<S>) -> HostInternal {
match host {
@@ -312,17 +312,23 @@ impl Url {
self.serialization
}

#[doc(hidden)]
pub fn assert_invariants(&self) {
self.assert_invariants_result().unwrap()
}

/// For internal testing, not part of the public API.
///
/// Methods of the `Url` struct assume a number of invariants.
/// This checks each of these invariants and panic if one is not met.
/// This is for testing rust-url itself.
#[doc(hidden)]
pub fn assert_invariants(&self) {
pub fn assert_invariants_result(&self) -> Result<(), String> {
macro_rules! assert {
($x: expr) => {
if !$x {
panic!("!( {} ) for URL {:?}", stringify!($x), self.serialization)
return Err(format!("!( {} ) for URL {:?}",
stringify!($x), self.serialization))
}
}
}
@@ -333,8 +339,9 @@ impl Url {
let a = $a;
let b = $b;
if a != b {
panic!("{:?} != {:?} ({} != {}) for URL {:?}",
a, b, stringify!($a), stringify!($b), self.serialization)
return Err(format!("{:?} != {:?} ({} != {}) for URL {:?}",
a, b, stringify!($a), stringify!($b),
self.serialization))
}
}
}
@@ -415,6 +422,7 @@ impl Url {
assert_eq!(self.path_start, other.path_start);
assert_eq!(self.query_start, other.query_start);
assert_eq!(self.fragment_start, other.fragment_start);
Ok(())
}

/// Return the origin of this URL (https://url.spec.whatwg.org/#origin)
@@ -1506,20 +1514,51 @@ impl rustc_serialize::Decodable for Url {
///
/// This implementation is only available if the `serde` Cargo feature is enabled.
#[cfg(feature="serde")]
#[deny(unused)]
impl serde::Serialize for Url {
fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error> where S: serde::Serializer {
format!("{}", self).serialize(serializer)
// Destructuring first lets us ensure that adding or removing fields forces this method
// to be updated
let Url { ref serialization, ref scheme_end,
ref username_end, ref host_start,
ref host_end, ref host, ref port,
ref path_start, ref query_start,
ref fragment_start} = *self;
(serialization, scheme_end, username_end,
host_start, host_end, host, port, path_start,
query_start, fragment_start).serialize(serializer)
}
}

/// Deserializes this URL from a `serde` stream.
///
/// This implementation is only available if the `serde` Cargo feature is enabled.
#[cfg(feature="serde")]
#[deny(unused)]
impl serde::Deserialize for Url {
fn deserialize<D>(deserializer: &mut D) -> Result<Url, D::Error> where D: serde::Deserializer {
let string_representation: String = try!(serde::Deserialize::deserialize(deserializer));
Ok(Url::parse(&string_representation).unwrap())
use serde::{Deserialize, Error};
let (serialization, scheme_end, username_end,
host_start, host_end, host, port, path_start,
query_start, fragment_start) = Deserialize::deserialize(deserializer)?;
let url = Url {
serialization: serialization,
scheme_end: scheme_end,
username_end: username_end,
host_start: host_start,
host_end: host_end,
host: host,
port: port,
path_start: path_start,
query_start: query_start,
fragment_start: fragment_start
};
#[cfg(debug_assertions)] {

This comment has been minimized.

@nox

nox Dec 18, 2016

Member

Without debug_assertions, type errors in that block won't be emitted. Do if cfg!(debug_assertions) instead.

if let Err(s) = url.assert_invariants_result() {
return Err(Error::invalid_value(&s))
}
}
Ok(url)
}
}

@@ -8,13 +8,27 @@

//! Data-driven tests

extern crate bincode;
extern crate rustc_serialize;
#[cfg(feature="serde")]
extern crate serde;
extern crate test;
extern crate url;

use rustc_serialize::json::{self, Json};
use url::{Url, quirks};

fn assert_invariants(url: &Url) {
url.assert_invariants();
#[cfg(feature="serde")] {
use bincode::SizeLimit;
use bincode::serde::{deserialize, serialize};
let bytes = serialize(url, SizeLimit::Infinite).unwrap();
let new_url: Url = deserialize(&bytes).unwrap();
assert_eq!(url, &new_url);
}
}


fn run_parsing(input: String, base: String, expected: Result<ExpectedAttributes, ()>) {
let base = match Url::parse(&base) {
@@ -28,7 +42,7 @@ fn run_parsing(input: String, base: String, expected: Result<ExpectedAttributes,
(Ok(_), Err(())) => panic!("Expected a parse error for URL {:?}", input),
};

url.assert_invariants();
assert_invariants(&url);

macro_rules! assert_eq {
($expected: expr, $got: expr) => {
@@ -144,11 +158,11 @@ fn collect_setters<F>(add_test: &mut F) where F: FnMut(String, test::TestFn) {
let mut expected = test.take("expected").unwrap();
add_test(name, test::TestFn::dyn_test_fn(move || {
let mut url = Url::parse(&href).unwrap();
url.assert_invariants();
assert_invariants(&url);
let _ = quirks::$setter(&mut url, &new_value);
assert_attributes!(url, expected,
href protocol username password host hostname port pathname search hash);
url.assert_invariants();
assert_invariants(&url);
}))
}
}}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.