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 a one-token cache #171

Merged
merged 15 commits into from
Jul 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]

name = "cssparser"
version = "0.17.0"
version = "0.18.0"
authors = [ "Simon Sapin <simon.sapin@exyr.org>" ]

description = "Rust implementation of CSS Syntax Level 3"
Expand Down
41 changes: 18 additions & 23 deletions src/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ impl Color {
///
/// FIXME(#2) Deprecated CSS2 System Colors are not supported yet.
pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Color, BasicParseError<'i>> {
let token = input.next()?;
// FIXME: remove clone() when lifetimes are non-lexical
let token = input.next()?.clone();
match token {
Token::Hash(ref value) | Token::IDHash(ref value) => {
Color::parse_hash(value.as_bytes())
Expand Down Expand Up @@ -422,21 +423,17 @@ fn parse_color_function<'i, 't>(name: &str, arguments: &mut Parser<'i, 't>) -> R
if uses_commas {
arguments.expect_comma()?;
} else {
match arguments.next()? {
Token::Delim('/') => {},
t => return Err(BasicParseError::UnexpectedToken(t)),
};
arguments.expect_delim('/')?;
};
let token = arguments.next()?;
match token {
match *arguments.next()? {
Token::Number { value: v, .. } => {
clamp_unit_f32(v)
}
Token::Percentage { unit_value: v, .. } => {
clamp_unit_f32(v)
}
t => {
return Err(BasicParseError::UnexpectedToken(t))
ref t => {
return Err(BasicParseError::UnexpectedToken(t.clone()))
}
}
} else {
Expand All @@ -457,10 +454,10 @@ fn parse_rgb_components_rgb<'i, 't>(arguments: &mut Parser<'i, 't>) -> Result<(u

// Either integers or percentages, but all the same type.
// https://drafts.csswg.org/css-color/#rgb-functions
match arguments.next()? {
match arguments.next()?.clone() {
Token::Number { value: v, .. } => {
red = clamp_floor_256_f32(v);
green = clamp_floor_256_f32(match arguments.next()? {
green = clamp_floor_256_f32(match arguments.next()?.clone() {
Token::Number { value: v, .. } => v,
Token::Comma => {
uses_commas = true;
Expand All @@ -475,7 +472,7 @@ fn parse_rgb_components_rgb<'i, 't>(arguments: &mut Parser<'i, 't>) -> Result<(u
}
Token::Percentage { unit_value, .. } => {
red = clamp_unit_f32(unit_value);
green = clamp_unit_f32(match arguments.next()? {
green = clamp_unit_f32(match arguments.next()?.clone() {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm... This is kind of unfortunate, because it means that before we created a token, and now we need to create two copies of it... Does it happen too often? I wonder if that's why parsing a stylesheet becomes slower?

Copy link
Member Author

Choose a reason for hiding this comment

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

Parsing initially became slower when Parser::next would clone every single token in order to put in the cache. Returning &Token removed these clones. In a minority of cases like here, the callers had to do these clones themselves instead. Note though that Token::clone never allocates. (Refcounting is used when a token contains an allocation.)

I don’t know the reason for the more recent regression. I’ll investigate next week.

Token::Percentage { unit_value, .. } => unit_value,
Token::Comma => {
uses_commas = true;
Expand All @@ -498,28 +495,26 @@ fn parse_rgb_components_hsl<'i, 't>(arguments: &mut Parser<'i, 't>) -> Result<(u
let mut uses_commas = false;
// Hue given as an angle
// https://drafts.csswg.org/css-values/#angles
let token = arguments.next()?;
let hue_degrees = match token {
Token::Number { value: v, .. } => Ok(v),
let hue_degrees = match *arguments.next()? {
Token::Number { value: v, .. } => v,
Token::Dimension { value: v, ref unit, .. } => {
match_ignore_ascii_case! { &*unit,
"deg" => Ok(v),
"grad" => Ok(v * 360. / 400.),
"rad" => Ok(v * 360. / (2. * PI)),
"turn" => Ok(v * 360.),
_ => Err(()),
"deg" => v,
"grad" => v * 360. / 400.,
"rad" => v * 360. / (2. * PI),
"turn" => v * 360.,
_ => return Err(BasicParseError::UnexpectedToken(Token::Ident(unit.clone()))),
}
}
t => return Err(BasicParseError::UnexpectedToken(t))
ref t => return Err(BasicParseError::UnexpectedToken(t.clone()))
};
let hue_degrees = hue_degrees.map_err(|()| BasicParseError::UnexpectedToken(token))?;
// Subtract an integer before rounding, to avoid some rounding errors:
let hue_normalized_degrees = hue_degrees - 360. * (hue_degrees / 360.).floor();
let hue = hue_normalized_degrees / 360.;

// Saturation and lightness are clamped to 0% ... 100%
// https://drafts.csswg.org/css-color/#the-hsl-notation
let saturation = match arguments.next()? {
let saturation = match arguments.next()?.clone() {
Token::Percentage { unit_value, .. } => unit_value,
Token::Comma => {
uses_commas = true;
Expand Down
246 changes: 0 additions & 246 deletions src/compact_cow_str.rs

This file was deleted.

Loading