From d0a97467f48c9f5afec27bbc5d956e13c446100f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 7 Mar 2025 01:00:54 +0100 Subject: [PATCH] Improve boundary classification (#17005) This PR cleans up the boundary character checking by using similar classification techniques as we used for other classification problems. For starters, this moves the boundary related items to its own file, next we setup the classification enum. Last but not least, we removed `}` as an _after_ boundary character, and instead handle that situation in the Ruby pre processor where we need it. This means the `%w{flex}` will still work in Ruby files. --- This PR is a followup for https://github.com/tailwindlabs/tailwindcss/pull/17001, the main goal is to clean up some of the boundary character checking code. The other big improvement is performance. Changing the boundary character checking to use a classification instead results in: Took the best score of 10 runs each: ```diff - CandidateMachine: Throughput: 311.96 MB/s + CandidateMachine: Throughput: 333.52 MB/s ``` So a ~20MB/s improvement. # Test plan 1. Existing tests should pass. Due to the removal of `}` as an after boundary character, some tests are updated. 2. Added new tests to ensure the Ruby pre processor still works as expected. --------- Co-authored-by: Jordan Pittman --- crates/classification-macros/src/lib.rs | 6 + crates/oxide/src/extractor/boundary.rs | 104 ++++++++++++++++++ .../oxide/src/extractor/candidate_machine.rs | 51 +-------- crates/oxide/src/extractor/mod.rs | 1 + .../src/extractor/named_utility_machine.rs | 7 +- .../src/extractor/pre_processors/ruby.rs | 43 +++++++- crates/oxide/src/extractor/utility_machine.rs | 5 +- 7 files changed, 156 insertions(+), 61 deletions(-) create mode 100644 crates/oxide/src/extractor/boundary.rs diff --git a/crates/classification-macros/src/lib.rs b/crates/classification-macros/src/lib.rs index 4e10fc5ad..7bee55595 100644 --- a/crates/classification-macros/src/lib.rs +++ b/crates/classification-macros/src/lib.rs @@ -107,6 +107,12 @@ pub fn classify_bytes_derive(input: TokenStream) -> TokenStream { #enum_name::TABLE[byte as usize] } } + + impl From<&u8> for #enum_name { + fn from(byte: &u8) -> Self { + #enum_name::TABLE[*byte as usize] + } + } }; TokenStream::from(expanded) diff --git a/crates/oxide/src/extractor/boundary.rs b/crates/oxide/src/extractor/boundary.rs new file mode 100644 index 000000000..3d13d895c --- /dev/null +++ b/crates/oxide/src/extractor/boundary.rs @@ -0,0 +1,104 @@ +use classification_macros::ClassifyBytes; + +use crate::extractor::Span; + +#[inline(always)] +pub fn is_valid_before_boundary(c: &u8) -> bool { + matches!(c.into(), Class::Common | Class::Before) +} + +#[inline(always)] +pub fn is_valid_after_boundary(c: &u8) -> bool { + matches!(c.into(), Class::Common | Class::After) +} + +#[inline(always)] +pub fn has_valid_boundaries(span: &Span, input: &[u8]) -> bool { + let before = { + if span.start == 0 { + b'\0' + } else { + input[span.start - 1] + } + }; + + let after = { + if span.end >= input.len() - 1 { + b'\0' + } else { + input[span.end + 1] + } + }; + + // Ensure the span has valid boundary characters before and after + is_valid_before_boundary(&before) && is_valid_after_boundary(&after) +} + +#[derive(Debug, Clone, Copy, ClassifyBytes)] +enum Class { + // Whitespace, e.g.: + // + // ``` + //
+ // ^ ^ + // ``` + #[bytes(b'\t', b'\n', b'\x0C', b'\r', b' ')] + // Quotes, e.g.: + // + // ``` + //
+ // ^ ^ + // ``` + #[bytes(b'"', b'\'', b'`')] + // End of the input, e.g.: + // + // ``` + // flex + // ^ + // ``` + #[bytes(b'\0')] + Common, + + // Angular like attributes, e.g.: + // + // ```` + // [class.foo] + // ^ + // ``` + #[bytes(b'.')] + // Twig-like templating languages, e.g.: + // + // ``` + //
+ // ^ + // ``` + #[bytes(b'}')] + Before, + + // Clojure and Angular like languages, e.g.: + // ``` + // [:div.p-2] + // ^ + // [class.foo] + // ^ + // ``` + #[bytes(b']')] + // Twig like templating languages, e.g.: + // + // ``` + //
+ // ^ + // ``` + #[bytes(b'{')] + // Svelte like attributes, e.g.: + // + // ``` + //
+ // ^ + // ``` + #[bytes(b'=')] + After, + + #[fallback] + Other, +} diff --git a/crates/oxide/src/extractor/candidate_machine.rs b/crates/oxide/src/extractor/candidate_machine.rs index e998c17d5..1d92ea153 100644 --- a/crates/oxide/src/extractor/candidate_machine.rs +++ b/crates/oxide/src/extractor/candidate_machine.rs @@ -1,4 +1,5 @@ use crate::cursor; +use crate::extractor::boundary::{has_valid_boundaries, is_valid_before_boundary}; use crate::extractor::machine::{Machine, MachineState}; use crate::extractor::utility_machine::UtilityMachine; use crate::extractor::variant_machine::VariantMachine; @@ -176,56 +177,6 @@ impl CandidateMachine { } } -/// A candidate must be preceded or followed by any of these characters -/// E.g.: `
` -/// ^ Valid for `flex` -/// ^ Invalid for `div` -#[inline(always)] -fn is_valid_common_boundary(c: &u8) -> bool { - matches!( - c, - b'\t' | b'\n' | b'\x0C' | b'\r' | b' ' | b'"' | b'\'' | b'`' | b'\0' - ) -} - -/// A candidate must be preceded by any of these characters. -#[inline(always)] -fn is_valid_before_boundary(c: &u8) -> bool { - is_valid_common_boundary(c) || matches!(c, b'.' | b'}') -} - -/// A candidate must be followed by any of these characters. -/// -/// E.g.: `[class.foo]` Angular -/// E.g.: `
` Svelte -/// ^ -#[inline(always)] -pub fn is_valid_after_boundary(c: &u8) -> bool { - is_valid_common_boundary(c) || matches!(c, b'}' | b']' | b'=' | b'{') -} - -#[inline(always)] -fn has_valid_boundaries(span: &Span, input: &[u8]) -> bool { - let before = { - if span.start == 0 { - b'\0' - } else { - input[span.start - 1] - } - }; - - let after = { - if span.end >= input.len() - 1 { - b'\0' - } else { - input[span.end + 1] - } - }; - - // Ensure the span has valid boundary characters before and after - is_valid_before_boundary(&before) && is_valid_after_boundary(&after) -} - #[cfg(test)] mod tests { use super::CandidateMachine; diff --git a/crates/oxide/src/extractor/mod.rs b/crates/oxide/src/extractor/mod.rs index a3e16c6ca..baf983831 100644 --- a/crates/oxide/src/extractor/mod.rs +++ b/crates/oxide/src/extractor/mod.rs @@ -8,6 +8,7 @@ use std::fmt; pub mod arbitrary_property_machine; pub mod arbitrary_value_machine; pub mod arbitrary_variable_machine; +mod boundary; pub mod bracket_stack; pub mod candidate_machine; pub mod css_variable_machine; diff --git a/crates/oxide/src/extractor/named_utility_machine.rs b/crates/oxide/src/extractor/named_utility_machine.rs index a4d3b9de0..ab09f95bc 100644 --- a/crates/oxide/src/extractor/named_utility_machine.rs +++ b/crates/oxide/src/extractor/named_utility_machine.rs @@ -1,7 +1,7 @@ use crate::cursor; use crate::extractor::arbitrary_value_machine::ArbitraryValueMachine; use crate::extractor::arbitrary_variable_machine::ArbitraryVariableMachine; -use crate::extractor::candidate_machine::is_valid_after_boundary; +use crate::extractor::boundary::is_valid_after_boundary; use crate::extractor::machine::{Machine, MachineState}; use classification_macros::ClassifyBytes; @@ -485,10 +485,7 @@ mod tests { vec!["let", "classes", "true"], ), // Inside an object (no spaces, key) - ( - r#"let classes = {'{}':true};"#, - vec!["let", "classes", "true"], - ), + (r#"let classes = {'{}':true};"#, vec!["let", "classes"]), // Inside an object (value) ( r#"let classes = { primary: '{}' };"#, diff --git a/crates/oxide/src/extractor/pre_processors/ruby.rs b/crates/oxide/src/extractor/pre_processors/ruby.rs index 09e775b2b..180101479 100644 --- a/crates/oxide/src/extractor/pre_processors/ruby.rs +++ b/crates/oxide/src/extractor/pre_processors/ruby.rs @@ -27,6 +27,7 @@ impl PreProcessor for Ruby { let boundary = match cursor.curr { b'[' => b']', b'(' => b')', + b'{' => b'}', _ => { cursor.advance(); continue; @@ -54,12 +55,12 @@ impl PreProcessor for Ruby { } // Start of a nested bracket - b'[' | b'(' => { + b'[' | b'(' | b'{' => { bracket_stack.push(cursor.curr); } // End of a nested bracket - b']' | b')' if !bracket_stack.is_empty() => { + b']' | b')' | b'}' if !bracket_stack.is_empty() => { if !bracket_stack.pop(cursor.curr) { // Unbalanced cursor.advance(); @@ -98,6 +99,12 @@ mod tests { "%w[flex data-[state=pending]:bg-[#0088cc] flex-col]", "%w flex data-[state=pending]:bg-[#0088cc] flex-col ", ), + // %w{…} + ("%w{flex px-2.5}", "%w flex px-2.5 "), + ( + "%w{flex data-[state=pending]:bg-(--my-color) flex-col}", + "%w flex data-[state=pending]:bg-(--my-color) flex-col ", + ), // %w(…) ("%w(flex px-2.5)", "%w flex px-2.5 "), ( @@ -114,4 +121,36 @@ mod tests { Ruby::test(input, expected); } } + + #[test] + fn test_ruby_extraction() { + for (input, expected) in [ + // %w[…] + ("%w[flex px-2.5]", vec!["flex", "px-2.5"]), + ("%w[px-2.5 flex]", vec!["flex", "px-2.5"]), + ("%w[2xl:flex]", vec!["2xl:flex"]), + ( + "%w[flex data-[state=pending]:bg-[#0088cc] flex-col]", + vec!["flex", "data-[state=pending]:bg-[#0088cc]", "flex-col"], + ), + // %w{…} + ("%w{flex px-2.5}", vec!["flex", "px-2.5"]), + ("%w{px-2.5 flex}", vec!["flex", "px-2.5"]), + ("%w{2xl:flex}", vec!["2xl:flex"]), + ( + "%w{flex data-[state=pending]:bg-(--my-color) flex-col}", + vec!["flex", "data-[state=pending]:bg-(--my-color)", "flex-col"], + ), + // %w(…) + ("%w(flex px-2.5)", vec!["flex", "px-2.5"]), + ("%w(px-2.5 flex)", vec!["flex", "px-2.5"]), + ("%w(2xl:flex)", vec!["2xl:flex"]), + ( + "%w(flex data-[state=pending]:bg-(--my-color) flex-col)", + vec!["flex", "data-[state=pending]:bg-(--my-color)", "flex-col"], + ), + ] { + Ruby::test_extract_contains(input, expected); + } + } } diff --git a/crates/oxide/src/extractor/utility_machine.rs b/crates/oxide/src/extractor/utility_machine.rs index 5f8c74aa7..26b7019c2 100644 --- a/crates/oxide/src/extractor/utility_machine.rs +++ b/crates/oxide/src/extractor/utility_machine.rs @@ -312,10 +312,7 @@ mod tests { vec!["let", "classes", "true"], ), // Inside an object (no spaces, key) - ( - r#"let classes = {'{}':true};"#, - vec!["let", "classes", "true"], - ), + (r#"let classes = {'{}':true};"#, vec!["let", "classes"]), // Inside an object (value) ( r#"let classes = { primary: '{}' };"#,