Skip to content

Comments

perf(parser): use range checks for is_any_keyword() and is_number()#14410

Merged
graphite-app[bot] merged 1 commit intomainfrom
perf/range-check-optimizations
Oct 7, 2025
Merged

perf(parser): use range checks for is_any_keyword() and is_number()#14410
graphite-app[bot] merged 1 commit intomainfrom
perf/range-check-optimizations

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Oct 7, 2025

Summary

Replace multi-function calls and multiple enum variant checks with simple range checks, reducing assembly instructions in hot paths.

Changes

is_any_keyword()

Before: Called 4 separate functions checking 70+ enum variants:

  • is_reserved_keyword() - 38 variants
  • is_contextual_keyword() - 39 variants
  • is_strict_mode_contextual_keyword() - 8 variants
  • is_future_reserved_keyword() - 7 variants

After: Single range check Await..=Yield since all keywords are contiguous in the enum

is_number()

Before: Matched 11 separate enum variants
After: Single range check Decimal..=HexBigInt since numeric literals are contiguous

Assembly Analysis

Before (with scattered checks)

mov   x8, #992              ; Load bitmask constant
movk  x8, #992, lsl #16     ; More bitmask setup
movk  x8, #240, lsl #32     ; Even more bitmask setup
lsr   x8, x8, x0            ; Shift by kind value
and   w0, w8, #0x1          ; Extract result bit

5 instructions with complex constant loading

After (with range check)

and   w8, w0, #0xff         ; Extract byte
sub   w8, w8, #5            ; Subtract range start
cmp   w8, #39               ; Compare to range size
cset  w0, lo                ; Set result

4 instructions with simple arithmetic

Performance Impact

  • 20% fewer instructions (5 → 4)
  • Simpler logic = better CPU pipeline utilization
  • No complex constants = smaller code size
  • Better branch prediction with single comparison

This is particularly important because:

  • is_any_keyword() is called from advance() on every single token
  • This is one of the hottest code paths in the entire parser

Testing

Added unit tests to verify that:

  • All keywords remain contiguous in the enum (Await..=Yield)
  • All numeric literals remain contiguous (Decimal..=HexBigInt)

These tests will catch any future enum reordering that would break the optimization.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings October 7, 2025 11:27
@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 7, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

@github-actions github-actions bot added A-parser Area - Parser C-performance Category - Solution not expected to change functional behavior, only performance labels Oct 7, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR optimizes two hot path functions in the lexer by replacing multiple enum variant checks with efficient range checks. The optimization leverages the contiguous layout of keywords and numeric literals in the enum to reduce assembly instructions.

  • Replaced multi-function calls in is_any_keyword() with a single range check
  • Simplified is_number() from matching 11 variants to a range check
  • Added unit tests to verify enum contiguity assumptions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 7, 2025

CodSpeed Performance Report

Merging #14410 will not alter performance

Comparing perf/range-check-optimizations (2b6a3b4) with main (1f5167a)

Summary

✅ 37 untouched

@Boshen Boshen force-pushed the perf/range-check-optimizations branch 3 times, most recently from 05689d7 to 9598a9e Compare October 7, 2025 11:56
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Oct 7, 2025
Copy link
Member Author

Boshen commented Oct 7, 2025

Merge activity

…14410)

## Summary

Replace multi-function calls and multiple enum variant checks with simple range checks, reducing assembly instructions in hot paths.

## Changes

### `is_any_keyword()`
**Before**: Called 4 separate functions checking 70+ enum variants:
- `is_reserved_keyword()` - 38 variants
- `is_contextual_keyword()` - 39 variants
- `is_strict_mode_contextual_keyword()` - 8 variants
- `is_future_reserved_keyword()` - 7 variants

**After**: Single range check `Await..=Yield` since all keywords are contiguous in the enum

### `is_number()`
**Before**: Matched 11 separate enum variants
**After**: Single range check `Decimal..=HexBigInt` since numeric literals are contiguous

## Assembly Analysis

### Before (with scattered checks)
```asm
mov   x8, #992              ; Load bitmask constant
movk  x8, #992, lsl #16     ; More bitmask setup
movk  x8, #240, lsl #32     ; Even more bitmask setup
lsr   x8, x8, x0            ; Shift by kind value
and   w0, w8, #0x1          ; Extract result bit
```
**5 instructions** with complex constant loading

### After (with range check)
```asm
and   w8, w0, #0xff         ; Extract byte
sub   w8, w8, #5            ; Subtract range start
cmp   w8, #39               ; Compare to range size
cset  w0, lo                ; Set result
```
**4 instructions** with simple arithmetic

## Performance Impact

- **20% fewer instructions** (5 → 4)
- **Simpler logic** = better CPU pipeline utilization
- **No complex constants** = smaller code size
- **Better branch prediction** with single comparison

This is particularly important because:
- `is_any_keyword()` is called from `advance()` on **every single token**
- This is one of the hottest code paths in the entire parser

## Testing

Added unit tests to verify that:
- All keywords remain contiguous in the enum (`Await..=Yield`)
- All numeric literals remain contiguous (`Decimal..=HexBigInt`)

These tests will catch any future enum reordering that would break the optimization.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app bot force-pushed the perf/range-check-optimizations branch from 9598a9e to 2b6a3b4 Compare October 7, 2025 12:02
@graphite-app graphite-app bot merged commit 2b6a3b4 into main Oct 7, 2025
27 checks passed
@graphite-app graphite-app bot deleted the perf/range-check-optimizations branch October 7, 2025 12:08
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-parser Area - Parser C-performance Category - Solution not expected to change functional behavior, only performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant