Skip to content

Comments

perf(formatter): use VecDeque for member chain groups#18094

Merged
graphite-app[bot] merged 1 commit intomainfrom
perf/formatter-vecdeque-member-chain
Jan 19, 2026
Merged

perf(formatter): use VecDeque for member chain groups#18094
graphite-app[bot] merged 1 commit intomainfrom
perf/formatter-vecdeque-member-chain

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Jan 16, 2026

Summary

  • Replace Vec with VecDeque for TailChainGroups storage
  • pop_first() now uses pop_front() which is O(1) instead of O(n) remove(0)

Changes

Operation Before After
pop_first() remove(0) - O(n) shift pop_front() - O(1)
first() first() front()
last() last() back()
push push() push_back()

Impact

Improves performance when formatting long member chains like a.b().c().d().e() where the first group may be merged with the head.

Test plan

  • cargo test -p oxc_formatter passes (144 tests)
  • Prettier conformance unchanged (97.77% js, 96.37% ts)

🤖 Generated with Claude Code

@Boshen Boshen requested a review from Dunqing as a code owner January 16, 2026 16:14
Copilot AI review requested due to automatic review settings January 16, 2026 16:14
@github-actions github-actions bot added A-formatter Area - Formatter C-performance Category - Solution not expected to change functional behavior, only performance labels Jan 16, 2026
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 the formatter's member chain handling by replacing Vec with VecDeque for storing tail chain groups. The change improves the performance of pop_first() operations from O(n) to O(1), which benefits formatting of long member chains.

Changes:

  • Replaced Vec with VecDeque for TailChainGroups and MemberChainGroupsBuilder storage
  • Updated all collection operations to use VecDeque-specific methods (push_back, pop_front, front, back)
  • Refactored any_except_last_will_break() to use iterator-based approach compatible with VecDeque

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 16, 2026

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing perf/formatter-vecdeque-member-chain (eef381a) with main (618c629)

Summary

✅ 38 untouched benchmarks
⏩ 7 skipped benchmarks1

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Jan 19, 2026
Copy link
Member

Dunqing commented Jan 19, 2026

Merge activity

## Summary

- Replace `Vec` with `VecDeque` for `TailChainGroups` storage
- `pop_first()` now uses `pop_front()` which is O(1) instead of O(n) `remove(0)`

## Changes

| Operation | Before | After |
|-----------|--------|-------|
| `pop_first()` | `remove(0)` - O(n) shift | `pop_front()` - O(1) |
| `first()` | `first()` | `front()` |
| `last()` | `last()` | `back()` |
| `push` | `push()` | `push_back()` |

## Impact

Improves performance when formatting long member chains like `a.b().c().d().e()` where the first group may be merged with the head.

## Test plan

- [x] `cargo test -p oxc_formatter` passes (144 tests)
- [x] Prettier conformance unchanged (97.77% js, 96.37% ts)

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app bot force-pushed the perf/formatter-vecdeque-member-chain branch from eef381a to 138637c Compare January 19, 2026 01:32
@graphite-app graphite-app bot merged commit 138637c into main Jan 19, 2026
21 checks passed
@graphite-app graphite-app bot deleted the perf/formatter-vecdeque-member-chain branch January 19, 2026 01:38
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-formatter Area - Formatter 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.

2 participants