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,