Skip to content

perf(oxfmt): make time measurement conditional#16634

Merged
graphite-app[bot] merged 1 commit intomainfrom
perf-oxfmt-conditional-timing
Dec 9, 2025
Merged

perf(oxfmt): make time measurement conditional#16634
graphite-app[bot] merged 1 commit intomainfrom
perf-oxfmt-conditional-timing

Conversation

@Boshen
Copy link
Copy Markdown
Member

@Boshen Boshen commented Dec 9, 2025

Summary

  • Make time measurement conditional based on output mode to reduce unnecessary overhead
  • Instant::now() is only called when using --check mode
  • Elapsed time calculation moved to only when it's actually displayed

Test plan

  • Verify timing information still appears correctly in --check mode
  • Confirm no timing overhead in --write mode
  • Confirm no timing overhead in --list-different mode
  • Run cargo check -p oxfmt passes
  • Run just fmt completes successfully

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings December 9, 2025 07:43
@github-actions github-actions bot added A-cli Area - CLI A-formatter Area - Formatter C-performance Category - Solution not expected to change functional behavior, only performance labels Dec 9, 2025
Copy link
Copy Markdown
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 performance in oxfmt by making time measurement conditional on the output mode. The Instant::now() call is now only executed in --check mode where timing information is actually displayed, eliminating unnecessary overhead in --write and --list-different modes.

Key changes:

  • Conditional time measurement using lazy evaluation with .then(Instant::now)
  • Moved elapsed time calculation from unconditional (line 74) to only when needed in Check mode (line 94)

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

@Boshen Boshen requested a review from leaysgur December 9, 2025 08:21
@leaysgur leaysgur added the 0-merge Merge with Graphite Merge Queue label Dec 9, 2025
Copy link
Copy Markdown
Member

leaysgur commented Dec 9, 2025

Merge activity

## Summary

- Make time measurement conditional based on output mode to reduce unnecessary overhead
- `Instant::now()` is only called when using `--check` mode
- Elapsed time calculation moved to only when it's actually displayed

## Test plan

- [ ] Verify timing information still appears correctly in `--check` mode
- [ ] Confirm no timing overhead in `--write` mode
- [ ] Confirm no timing overhead in `--list-different` mode
- [ ] Run `cargo check -p oxfmt` passes
- [ ] Run `just fmt` completes successfully

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app bot force-pushed the perf-oxfmt-conditional-timing branch from 1c50560 to 10b4f9f Compare December 9, 2025 09:00
@graphite-app graphite-app bot merged commit 10b4f9f into main Dec 9, 2025
18 checks passed
@graphite-app graphite-app bot deleted the perf-oxfmt-conditional-timing branch December 9, 2025 09:06
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 9, 2025
Copilot AI pushed a commit that referenced this pull request Dec 10, 2025
## Summary

- Make time measurement conditional based on output mode to reduce unnecessary overhead
- `Instant::now()` is only called when using `--check` mode
- Elapsed time calculation moved to only when it's actually displayed

## Test plan

- [ ] Verify timing information still appears correctly in `--check` mode
- [ ] Confirm no timing overhead in `--write` mode
- [ ] Confirm no timing overhead in `--list-different` mode
- [ ] Run `cargo check -p oxfmt` passes
- [ ] Run `just fmt` completes successfully

🤖 Generated with [Claude Code](https://claude.com/claude-code)
taearls pushed a commit to taearls/oxc that referenced this pull request Dec 11, 2025
## Summary

- Make time measurement conditional based on output mode to reduce unnecessary overhead
- `Instant::now()` is only called when using `--check` mode
- Elapsed time calculation moved to only when it's actually displayed

## Test plan

- [ ] Verify timing information still appears correctly in `--check` mode
- [ ] Confirm no timing overhead in `--write` mode
- [ ] Confirm no timing overhead in `--list-different` mode
- [ ] Run `cargo check -p oxfmt` passes
- [ ] Run `just fmt` completes successfully

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI 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.

3 participants