perf: cache selector query#49
Conversation
WalkthroughThe changes in this pull request primarily involve updates to the Changes
Suggested reviewers
Poem
Warning Rate limit exceeded@alan-agius4 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
CodSpeed Performance ReportMerging #49 will degrade performances by 47.38%Comparing Summary
Benchmarks breakdown
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
packages/beasties/src/dom.ts (1)
344-345: Typographical errors in TODO comment.There are typographical errors in the comment; it should read: "We should probably move this cache as part of the class so that it's disposed with it."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/beasties/src/dom.ts(3 hunks)
🔇 Additional comments (1)
packages/beasties/src/dom.ts (1)
349-369: Verify thread safety of shared caches in asynchronous contexts.
If cachedQuerySelector is accessed concurrently in an asynchronous environment, using shared caches without proper synchronisation might cause race conditions. Ensure that the caches are either protected against concurrent modifications or that the function is used in a single-threaded context.
Run the following script to check for asynchronous uses of cachedQuerySelector:
alan-agius4
force-pushed
the
selector-query-cache
branch
from
405c030 to
bd7196f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/beasties/src/dom.ts(3 hunks)
🔇 Additional comments (2)
packages/beasties/src/dom.ts (2)
21-21: LGTM: Type import addition
The AttributeSelector type import is correctly added and necessary for the new caching implementation.
140-140: LGTM: Proper id setter implementation
The id setter correctly uses setAttribute, maintaining consistency with the className setter implementation.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
==========================================
+ Coverage 84.58% 84.74% +0.16%
==========================================
Files 7 7
Lines 1174 1193 +19
Branches 273 278 +5
==========================================
+ Hits 993 1011 +18
- Misses 181 182 +1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
alan-agius4
force-pushed
the
selector-query-cache
branch
3 times, most recently
from
01b7737 to
a871e1a
Compare
This operation is quite repetitive and intensive. However, it can be optimized through caching, as stylesheets rarely change between executions, and selectors are consistently evaluated in the same way.
alan-agius4
force-pushed
the
selector-query-cache
branch
from
a871e1a to
705ca8d
Compare
3 participants
This operation is quite repetitive and intensive. However, it can be optimized through caching, as stylesheets rarely change between executions, and selectors are consistently evaluated in the same way.
✨
Description by Callstackai
This PR introduces a caching mechanism for selector tokens to optimize the performance of querying nodes, along with a bug fix for a typo in the attribute setting functionality.
Diagrams of code changes
sequenceDiagram participant Client participant cachedQuerySelector participant selectorTokensCache participant parseRelevantSelectors participant selectorParser Client->>cachedQuerySelector: query(selector, node) cachedQuerySelector->>selectorTokensCache: get(selector) alt Cache Miss selectorTokensCache-->>cachedQuerySelector: undefined cachedQuerySelector->>parseRelevantSelectors: parseRelevantSelectors(selector) parseRelevantSelectors->>selectorParser: parse(selector) selectorParser-->>parseRelevantSelectors: tokens parseRelevantSelectors-->>cachedQuerySelector: relevantTokens cachedQuerySelector->>selectorTokensCache: set(selector, relevantTokens) else Cache Hit selectorTokensCache-->>cachedQuerySelector: cached tokens end alt Has Relevant Tokens cachedQuerySelector->>cachedQuerySelector: check class/id cache else No Relevant Tokens cachedQuerySelector->>cachedQuerySelector: fallback to selectOne end cachedQuerySelector-->>Client: resultFiles Changed
This PR includes files in programming languages that we currently do not support. We have not reviewed files with the extensions
.ts. See list of supported languages.Summary by CodeRabbit
New Features
Bug Fixes
These updates enhance the performance of attribute selector handling within the application.