perf(linter/no-redeclare): avoid repeated PHF hashmap lookups#18248
Conversation
github-actions
bot
added
A-linter
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Pull request overview
This PR optimizes the no-redeclare linter rule by caching the GLOBALS["builtin"] hashmap lookup outside the loop that iterates over all symbols. This avoids repeated expensive PHF hashmap lookups for each symbol when builtin_globals is enabled.
Changes:
- Cache
GLOBALS["builtin"]before the symbol iteration loop to avoid repeated hashmap lookups - Refactor the builtin check to use
is_some_andwith the cached value
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
Merge activity
|
Looking up `GLOBALS["builtin"]` is not so cheap (PHF hashmaps use quite a slow hash function). Do the lookup once only outside the loop, rather than repeatedly for each symbol. There is a more comprehensive solution - oxc-project/javascript-globals#149 - but this is should be an improvement in the meantime.
graphite-app
bot
force-pushed
the
om/01-19-perf_linter_no-redeclare_avoid_repeated_phf_hashmap_lookups
branch
from
a89ddea to
1a247f1
Compare
graphite-app
bot
deleted the
om/01-19-perf_linter_no-redeclare_avoid_repeated_phf_hashmap_lookups
branch
|
@camc314 You're on holiday! You can review PRs when you get back. If it was urgent, I'd have merged myself. |
# Oxlint ### ⚡ Performance - 1a247f1 linter/no-redeclare: Avoid repeated PHF hashmap lookups (#18248) (overlookmotel) ### 📚 Documentation - 8a294d5 oxfmt, oxlint: Update logo (#18242) (Dunqing) # Oxfmt ### 🐛 Bug Fixes - 864b4ab oxfmt: Use `format_config` for `external_options` override (#18252) (leaysgur) - 90ce4b7 oxfmt: Restore `customGroups` handling lost (#18243) (leaysgur) ### 📚 Documentation - 8a294d5 oxfmt, oxlint: Update logo (#18242) (Dunqing) Co-authored-by: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Looking up
GLOBALS["builtin"]is not so cheap (PHF hashmaps use quite a slow hash function). Do the lookup once only outside the loop, rather than repeatedly for each symbol.There is a more comprehensive solution - oxc-project/javascript-globals#149 - but this is should be an improvement in the meantime.