From 3e29da6a2ed0262b9f65959b6aa27fca6efe3a87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Fri, 10 Jul 2026 17:13:13 -0300 Subject: [PATCH] Optimize CSS selector matching with cached hashes Cache tag, ID, and class hashes in selector rules and widgets to avoid repeated string comparisons during selector matching. Store widget class hashes in a single-element SmallVector so the common zero and one-class cases remain allocation-free. Reorder UINode and UIWidget members to recover padding, keeping UIWidget at its original 1144-byte size. Add selector matching benchmarks and regression coverage for class mutation, compound selectors, and hash cache synchronization. Document the measured Phase 5 performance and memory results. --- .../eepp_css_selector_optimization_plan.md | 30 ++++++++ .../eepp/ui/css/stylesheetselectorrule.hpp | 3 + include/eepp/ui/uinode.hpp | 2 +- include/eepp/ui/uiwidget.hpp | 22 ++++++ src/benchmarks/inline_layout_benchmark.cpp | 72 +++++++++++++++++-- src/eepp/ui/css/stylesheetselectorrule.cpp | 24 ++++--- src/eepp/ui/uiwidget.cpp | 28 ++++++++ src/tests/unit_tests/uihtml_tests.cpp | 54 ++++++++++++++ 8 files changed, 220 insertions(+), 15 deletions(-) diff --git a/.agent/plans/eepp_css_selector_optimization_plan.md b/.agent/plans/eepp_css_selector_optimization_plan.md index 49269c460..0abd72a47 100644 --- a/.agent/plans/eepp_css_selector_optimization_plan.md +++ b/.agent/plans/eepp_css_selector_optimization_plan.md @@ -461,6 +461,36 @@ Use these counters to verify each later phase. # Phase 5: Add Hash-Based Tag/ID/Class Matching +**Status: Implemented** + +## Implementation State + +- Selector rules cache tag, ID, and sorted unique class hashes. +- `UIWidget` caches its tag hash and maintains sorted unique class hashes across every class + mutation API. +- Widget class hashes use `SmallVector`. The zero/one-class cases remain + allocation-free, while widgets with multiple classes use indirect storage. +- `StyleSheetSelectorRule::matches()` compares tag and ID hashes and checks all required class + hashes after decoding widget hash storage once per rule match. +- `UINode` and `UIWidget` members were reordered to reclaim alignment holes introduced or exposed + during the cache work. +- A focused selector benchmark and class-mutation regression test cover the implementation. + +## Measured Result + +- The release unit-test workload observed 30,431 destroyed widgets: 74.5% had no classes, 19.4% + had one, 5.9% had two, and approximately 0.1% had three or more. +- `sizeof( UIWidget )` is 1144 bytes, equal to the pre-Phase-5 size. `sizeof( UINode )` decreased + from 856 to 848 bytes. +- The focused release selector benchmark measured a median near 111.4 ms versus the Phase 3 + baseline near 115.1 ms, approximately a 3.2% matching improvement without net `UIWidget` + growth. +- The full release suite passes 734 tests with one skipped test. + +Future work should investigate atomized/interned class names as a separate architectural change. +That could replace both per-widget class strings and the hash cache, but requires explicit +ownership, collision, API, and cross-scene lifetime design. + ## Motivation `StyleSheetSelectorRule::matches()` currently compares strings repeatedly and class matching performs a nested linear search: diff --git a/include/eepp/ui/css/stylesheetselectorrule.hpp b/include/eepp/ui/css/stylesheetselectorrule.hpp index 86e45ff98..332b1b970 100644 --- a/include/eepp/ui/css/stylesheetselectorrule.hpp +++ b/include/eepp/ui/css/stylesheetselectorrule.hpp @@ -158,6 +158,9 @@ class EE_API StyleSheetSelectorRule { std::string mTagName; std::string mId; std::vector mClasses; + String::HashType mTagHash{ 0 }; + String::HashType mIdHash{ 0 }; + std::vector mClassHashes; std::vector mStructuralPseudoClasses; std::vector mStructuralSelectors; std::vector mAttributeSelectors; diff --git a/include/eepp/ui/uinode.hpp b/include/eepp/ui/uinode.hpp index cc6b071b4..a0ae5e8d9 100644 --- a/include/eepp/ui/uinode.hpp +++ b/include/eepp/ui/uinode.hpp @@ -1516,10 +1516,10 @@ class EE_API UINode : public Node { mutable UIBorderDrawable* mBorder; Vector2f mDragPoint; Color mSkinColor; + UIClip mClip; UISceneNode* mUISceneNode; Rectf mPadding; Rectf mPaddingPx; - UIClip mClip; std::string mMinWidthEq; std::string mMinHeightEq; std::string mMaxWidthEq; diff --git a/include/eepp/ui/uiwidget.hpp b/include/eepp/ui/uiwidget.hpp index 413d85a41..36fed3a33 100644 --- a/include/eepp/ui/uiwidget.hpp +++ b/include/eepp/ui/uiwidget.hpp @@ -1,6 +1,8 @@ #ifndef EE_UIUIWIDGET_HPP #define EE_UIUIWIDGET_HPP +#include +#include #include #include #include @@ -776,6 +778,20 @@ class EE_API UIWidget : public UINode { */ inline const std::vector& getStyleSheetClasses() const { return mClasses; } + /** @return Number of sorted unique CSS class hashes cached by this widget. */ + inline Uint32 getClassHashCount() const { return static_cast( mClassHashes.size() ); } + + /** @return True if all the sorted unique CSS class hashes are applied to this widget. */ + inline bool hasClassHashes( const std::vector& requiredHashes ) const { + const auto* hashes = mClassHashes.data(); + const auto* hashesEnd = hashes + mClassHashes.size(); + for ( auto hash : requiredHashes ) { + if ( !std::binary_search( hashes, hashesEnd, hash ) ) + return false; + } + return true; + } + /** * @brief Gets the parent element for CSS styling. * @@ -993,6 +1009,9 @@ class EE_API UIWidget : public UINode { */ inline const std::string& getElementTag() const { return mTag; } + /** @return The precomputed hash of the CSS element tag. */ + inline String::HashType getElementTagHash() const { return mTagHash; } + /** * @brief Pushes a state onto the widget's state stack. * @@ -1479,6 +1498,7 @@ class EE_API UIWidget : public UINode { SizePolicy mWidthPolicy; SizePolicy mHeightPolicy; PositionPolicy mLayoutPositionPolicy; + String::HashType mTagHash{ 0 }; UIWidget* mLayoutPositionPolicyWidget; int mAttributesTransactionCount; LayoutInvalidationFlags mPendingLayoutReasons{ 0 }; @@ -1486,6 +1506,7 @@ class EE_API UIWidget : public UINode { Uint32 mPseudoClasses{ 0 }; std::string mSkinName; std::vector mClasses; + SmallVector mClassHashes; String mTooltipText; mutable Float mMinIntrinsicWidth{ 0 }; mutable Float mMaxIntrinsicWidth{ 0 }; @@ -1493,6 +1514,7 @@ class EE_API UIWidget : public UINode { Uint8 mMarginAuto{ 0 }; void calculateAutoMargin(); + void rebuildClassHashes(); /** * @brief Default constructor. diff --git a/src/benchmarks/inline_layout_benchmark.cpp b/src/benchmarks/inline_layout_benchmark.cpp index f8fa3a409..f742ad33b 100644 --- a/src/benchmarks/inline_layout_benchmark.cpp +++ b/src/benchmarks/inline_layout_benchmark.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -25,8 +26,71 @@ static constexpr int numBoxes = 100; static constexpr int numSpansPerBox = 20; static constexpr int layoutIterations = 50; static constexpr int markdownFlushIterations = 1; +static constexpr int selectorMatchingIterations = 20000; static constexpr Float maxWidth = 800; +static int getSelectorMatchingIterations() { + if ( const char* env = std::getenv( "EE_CSS_SELECTOR_BENCH_ITERATIONS" ) ) { + Int32 val = selectorMatchingIterations; + if ( String::fromString( val, std::string( env ) ) ) + return eemax( 1, val ); + } + return selectorMatchingIterations; +} + +UTEST( Benchmark, CSSSelectorMatching ) { + Engine::instance()->createWindow( WindowSettings( 800, 600, "CSS selector bench", + WindowStyle::Default, WindowBackend::Default, + 32, {}, 1, false, true ) ); + UISceneNode* sceneNode = UISceneNode::New(); + SceneManager::instance()->add( sceneNode ); + + UIWidget* widget = UIWidget::NewWithTag( "div" ); + widget->setId( "selector-benchmark-target" ); + widget->setClasses( { "button", "primary", "toolbar-item", "interactive", "selected", "compact", + "theme-light", "enabled" } ); + widget->setParent( sceneNode->getRoot() ); + + static constexpr int selectorCount = 1024; + const int matchingIterations = getSelectorMatchingIterations(); + std::vector selectors; + selectors.reserve( selectorCount ); + + Clock constructionClock; + for ( int i = 0; i < selectorCount; ++i ) { + if ( i % 16 == 0 ) + selectors.emplace_back( "div.button.primary" ); + else if ( i % 16 == 1 ) + selectors.emplace_back( "#selector-benchmark-target.toolbar-item" ); + else + selectors.emplace_back( ".unmatched-class-" + String::toString( i ) ); + } + const Time constructionElapsed = constructionClock.getElapsedTime(); + + Uint64 matchCount = 0; + Clock matchingClock; + for ( int iteration = 0; iteration < matchingIterations; ++iteration ) { + for ( const auto& selector : selectors ) + matchCount += selector.select( widget, false ); + } + const Time matchingElapsed = matchingClock.getElapsedTime(); + + EXPECT_EQ( static_cast( matchingIterations ) * 128u, matchCount ); + UTEST_PRINT_INFO( String::format( "UIWidget size: %zu bytes", sizeof( UIWidget ) ).c_str() ); + UTEST_PRINT_INFO( + String::format( "StyleSheetSelectorRule size: %zu bytes", sizeof( StyleSheetSelectorRule ) ) + .c_str() ); + UTEST_PRINT_INFO( + String::format( "Selector construction: %lld us", constructionElapsed.asMicroseconds() ) + .c_str() ); + UTEST_PRINT_INFO( + String::format( "Selector matching: %lld us", matchingElapsed.asMicroseconds() ).c_str() ); + UTEST_PRINT_INFO( + String::format( "Selector calls: %d", selectorCount * matchingIterations ).c_str() ); + + Engine::destroySingleton(); +} + static int getMarkdownFlushIterations() { if ( const char* env = std::getenv( "EE_MARKDOWN_BENCH_FLUSH_ITERATIONS" ) ) { Int32 val = markdownFlushIterations; @@ -107,10 +171,10 @@ UTEST( Benchmark, MarkdownReadme ) { const std::string readmePath = "../../README.md"; std::string markdown; if ( !FileSystem::fileGet( readmePath, markdown ) ) { - UTEST_PRINT_INFO( - String::format( "Failed to load %s from cwd '%s', skipping benchmark", - readmePath.c_str(), FileSystem::getCurrentWorkingDirectory().c_str() ) - .c_str() ); + UTEST_PRINT_INFO( String::format( "Failed to load %s from cwd '%s', skipping benchmark", + readmePath.c_str(), + FileSystem::getCurrentWorkingDirectory().c_str() ) + .c_str() ); return; } diff --git a/src/eepp/ui/css/stylesheetselectorrule.cpp b/src/eepp/ui/css/stylesheetselectorrule.cpp index d3298d3e1..86f77858a 100644 --- a/src/eepp/ui/css/stylesheetselectorrule.cpp +++ b/src/eepp/ui/css/stylesheetselectorrule.cpp @@ -113,19 +113,23 @@ void StyleSheetSelectorRule::pushSelectorTypeIdentifier( TypeIdentifier selector switch ( selectorTypeIdentifier ) { case GLOBAL: String::toLowerInPlace( name ); + mTagHash = String::hash( name ); mTagName = std::move( name ); mSpecificity += SpecificityGlobal; break; case TAG: String::toLowerInPlace( name ); + mTagHash = String::hash( name ); mTagName = std::move( name ); mSpecificity += SpecificityTag; break; case CLASS: + mClassHashes.push_back( String::hash( name ) ); mClasses.push_back( std::move( name ) ); mSpecificity += SpecificityClass; break; case ID: + mIdHash = String::hash( name ); mId = std::move( name ); mSpecificity += SpecificityId; break; @@ -251,6 +255,10 @@ void StyleSheetSelectorRule::parseFragment( const std::string& selectorFragment if ( !mClasses.empty() ) mRequirementFlags |= Class; + std::sort( mClassHashes.begin(), mClassHashes.end() ); + mClassHashes.erase( std::unique( mClassHashes.begin(), mClassHashes.end() ), + mClassHashes.end() ); + if ( !mAttributeSelectors.empty() ) mRequirementFlags |= Attribute; @@ -312,7 +320,7 @@ bool StyleSheetSelectorRule::matches( UIWidget* element, const bool& applyPseudo if ( !mTagName.empty() ) { if ( mTagName != "*" ) { - if ( mTagName != element->getElementTag() ) { + if ( mTagHash != element->getElementTagHash() ) { return false; } else { flags |= TagName; @@ -329,23 +337,19 @@ bool StyleSheetSelectorRule::matches( UIWidget* element, const bool& applyPseudo } if ( !mId.empty() ) { - if ( mId != element->getId() ) { + if ( mIdHash != element->getIdHash() ) { return false; } else { flags |= Id; } } - if ( !mClasses.empty() ) { - const std::vector& elClasses = element->getStyleSheetClasses(); - if ( elClasses.empty() ) + if ( !mClassHashes.empty() ) { + if ( element->getClassHashCount() < mClassHashes.size() ) return false; - for ( const auto& cls : mClasses ) { - if ( std::find( elClasses.begin(), elClasses.end(), cls ) == elClasses.end() ) { - return false; - } - } + if ( !element->hasClassHashes( mClassHashes ) ) + return false; flags |= Class; } diff --git a/src/eepp/ui/uiwidget.cpp b/src/eepp/ui/uiwidget.cpp index 90bc00664..5f6aafe37 100644 --- a/src/eepp/ui/uiwidget.cpp +++ b/src/eepp/ui/uiwidget.cpp @@ -76,6 +76,7 @@ UIWidget::UIWidget( const std::string& tag ) : mWidthPolicy( SizePolicy::WrapContent ), mHeightPolicy( SizePolicy::WrapContent ), mLayoutPositionPolicy( PositionPolicy::None ), + mTagHash( String::hash( tag ) ), mLayoutPositionPolicyWidget( NULL ), mAttributesTransactionCount( 0 ) { mNodeFlags |= NODE_FLAG_WIDGET; @@ -1158,6 +1159,8 @@ void UIWidget::updatePseudoClasses() { UIWidget* UIWidget::resetClass() { if ( !mClasses.empty() ) { mClasses.clear(); + mClassHashes.clear(); + mClassHashes.shrink_to_fit(); if ( !isSceneNodeLoading() && !isLoadingState() ) { getUISceneNode()->invalidateStyle( this ); getUISceneNode()->invalidateStyleState( this ); @@ -1182,6 +1185,7 @@ UIWidget* UIWidget::setClass( const std::string& cls ) { getUISceneNode()->invalidateStyleState( this ); } } + rebuildClassHashes(); if ( oldClassesCount != mClasses.size() || isSet ) onClassChange(); } @@ -1202,6 +1206,7 @@ UIWidget* UIWidget::setClass( std::string&& cls ) { getUISceneNode()->invalidateStyleState( this ); } } + rebuildClassHashes(); if ( oldClassesCount != mClasses.size() || isSet ) onClassChange(); } @@ -1211,6 +1216,7 @@ UIWidget* UIWidget::setClass( std::string&& cls ) { UIWidget* UIWidget::setClasses( const std::vector& classes ) { if ( mClasses != classes ) { mClasses = classes; + rebuildClassHashes(); if ( !isSceneNodeLoading() && !isLoadingState() ) { getUISceneNode()->invalidateStyle( this ); @@ -1225,6 +1231,7 @@ UIWidget* UIWidget::setClasses( const std::vector& classes ) { UIWidget* UIWidget::addClass( const std::string& cls ) { if ( !cls.empty() && !hasClass( cls ) ) { mClasses.push_back( cls ); + rebuildClassHashes(); if ( !isSceneNodeLoading() && !isLoadingState() ) { getUISceneNode()->invalidateStyle( this ); @@ -1238,13 +1245,17 @@ UIWidget* UIWidget::addClass( const std::string& cls ) { UIWidget* UIWidget::addClasses( const std::vector& classes ) { if ( !classes.empty() ) { + bool classesChanged = false; for ( auto cit = classes.begin(); cit != classes.end(); ++cit ) { const std::string& cls = *cit; if ( !cls.empty() && !hasClass( cls ) ) { mClasses.push_back( cls ); + classesChanged = true; } } + if ( classesChanged ) + rebuildClassHashes(); if ( !isSceneNodeLoading() && !isLoadingState() ) { getUISceneNode()->invalidateStyle( this ); @@ -1259,6 +1270,7 @@ UIWidget* UIWidget::addClasses( const std::vector& classes ) { UIWidget* UIWidget::removeClass( const std::string& cls ) { if ( hasClass( cls ) ) { mClasses.erase( std::find( mClasses.begin(), mClasses.end(), cls ) ); + rebuildClassHashes(); if ( !isSceneNodeLoading() && !isLoadingState() ) { getUISceneNode()->invalidateStyle( this ); @@ -1272,6 +1284,7 @@ UIWidget* UIWidget::removeClass( const std::string& cls ) { UIWidget* UIWidget::removeClasses( const std::vector& classes ) { if ( !classes.empty() ) { + bool classesChanged = false; for ( auto cit = classes.begin(); cit != classes.end(); ++cit ) { const std::string& cls = *cit; @@ -1280,9 +1293,12 @@ UIWidget* UIWidget::removeClasses( const std::vector& classes ) { if ( found != mClasses.end() ) { mClasses.erase( found ); + classesChanged = true; } } } + if ( classesChanged ) + rebuildClassHashes(); if ( !isSceneNodeLoading() && !isLoadingState() ) { getUISceneNode()->invalidateStyle( this ); @@ -1323,6 +1339,7 @@ void UIWidget::setTooltipEnabled( bool enabled ) { void UIWidget::setElementTag( const std::string& tag ) { if ( mTag != tag ) { mTag = tag; + mTagHash = String::hash( tag ); // Some rules are going to be invalidated if the tag is changed mMinWidthEq = ""; mMinHeightEq = ""; @@ -1341,6 +1358,17 @@ const std::vector& UIWidget::getClasses() const { return mClasses; } +void UIWidget::rebuildClassHashes() { + mClassHashes.clear(); + mClassHashes.reserve( mClasses.size() ); + for ( const auto& cls : mClasses ) + mClassHashes.push_back( String::hash( cls ) ); + std::sort( mClassHashes.begin(), mClassHashes.end() ); + mClassHashes.erase( std::unique( mClassHashes.begin(), mClassHashes.end() ), + mClassHashes.end() ); + mClassHashes.shrink_to_fit(); +} + void UIWidget::pushState( const Uint32& State, bool emitEvent ) { if ( !( mState & ( 1 << State ) ) ) { mState |= 1 << State; diff --git a/src/tests/unit_tests/uihtml_tests.cpp b/src/tests/unit_tests/uihtml_tests.cpp index 001c28cd6..da212e3c6 100644 --- a/src/tests/unit_tests/uihtml_tests.cpp +++ b/src/tests/unit_tests/uihtml_tests.cpp @@ -2860,6 +2860,60 @@ UTEST( UIHTML, UniversalSelectorRequirements ) { Engine::destroySingleton(); } +UTEST( UIHTML, HashedSelectorMatchingAndClassMutation ) { + Engine::instance()->createWindow( WindowSettings( 1024, 768, "Hashed Selector Test", + WindowStyle::Default, WindowBackend::Default, + 32, {}, 1, false, true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + + UISceneNode* sceneNode = init_test_inline_block(); + sceneNode->loadLayoutFromString( HTMLFormatter::HTMLtoXML( R"html( +
+ )html" ) ); + + auto* target = sceneNode->getRoot()->find( "target" )->asType(); + ASSERT_TRUE( target != nullptr ); + EXPECT_EQ( String::hash( "div" ), target->getElementTagHash() ); + EXPECT_TRUE( target->hasClassHashes( { String::hash( "foo" ) } ) ); + EXPECT_TRUE( target->hasClassHashes( { String::hash( "bar" ) } ) ); + EXPECT_EQ( 2u, target->getClassHashCount() ); + + EXPECT_TRUE( StyleSheetSelector( ".foo" ).select( target, false ) ); + EXPECT_TRUE( StyleSheetSelector( ".foo.bar" ).select( target, false ) ); + EXPECT_TRUE( StyleSheetSelector( "div.foo" ).select( target, false ) ); + EXPECT_TRUE( StyleSheetSelector( "#target.foo" ).select( target, false ) ); + EXPECT_FALSE( StyleSheetSelector( "span.foo" ).select( target, false ) ); + EXPECT_FALSE( StyleSheetSelector( "#other.foo" ).select( target, false ) ); + + target->setClasses( { "alpha", "beta", "alpha" } ); + EXPECT_EQ( 2u, target->getClassHashCount() ); + EXPECT_TRUE( StyleSheetSelector( ".alpha.beta" ).select( target, false ) ); + target->removeClass( "alpha" ); + EXPECT_TRUE( StyleSheetSelector( ".alpha" ).select( target, false ) ); + target->removeClass( "alpha" ); + EXPECT_FALSE( StyleSheetSelector( ".alpha" ).select( target, false ) ); + target->addClasses( { "gamma", "delta" } ); + EXPECT_TRUE( StyleSheetSelector( ".beta.gamma.delta" ).select( target, false ) ); + target->removeClasses( { "beta", "delta" } ); + EXPECT_FALSE( target->hasClassHashes( { String::hash( "beta" ) } ) ); + EXPECT_FALSE( target->hasClassHashes( { String::hash( "delta" ) } ) ); + target->toggleClass( "gamma" ); + EXPECT_FALSE( StyleSheetSelector( ".gamma" ).select( target, false ) ); + target->toggleClass( "gamma" ); + EXPECT_TRUE( StyleSheetSelector( ".gamma" ).select( target, false ) ); + target->setClass( std::string( "moved" ) ); + EXPECT_TRUE( StyleSheetSelector( ".moved" ).select( target, false ) ); + target->resetClass(); + EXPECT_EQ( 0u, target->getClassHashCount() ); + + target->setElementTag( "section" ); + EXPECT_EQ( String::hash( "section" ), target->getElementTagHash() ); + EXPECT_TRUE( StyleSheetSelector( "section" ).select( target, false ) ); + EXPECT_FALSE( StyleSheetSelector( "div" ).select( target, false ) ); + + Engine::destroySingleton(); +} + UTEST( UIHTML, BlockList ) { Engine::instance()->createWindow( WindowSettings( 1024, 768, "Block List Test", WindowStyle::Default, WindowBackend::Default,