Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(linter): use parallel iterator directly instead of iter and parallel bridge #8831

Conversation

camchenry
Copy link
Contributor

@camchenry camchenry commented Feb 1, 2025

Experimenting with some threading optimization, I noticed that ParallelBridge says that using this can be less efficient than just using par_iter directly:

https://docs.rs/rayon/latest/rayon/iter/trait.ParallelBridge.html

This creates a “bridge” from a sequential iterator to a parallel one, by distributing its items across the Rayon thread pool. This has the advantage of being able to parallelize just about anything, but the resulting ParallelIterator can be less efficient than if you started with par_iter instead. However, it can still be useful for iterators that are difficult to parallelize by other means, like channels or file or network I/O.

May not reflect in codspeed since this needs multiple files to be noticeable. Going to benchmark locally first.

@github-actions github-actions bot added A-linter Area - Linter C-performance Category - Solution not expected to change functional behavior, only performance labels Feb 1, 2025
Copy link
Contributor Author


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.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@camchenry camchenry force-pushed the 02-01-perf_linter_use_parallel_iterator_directly_instead_of_iter_and_parallel_bridge branch from 76c9967 to 62585b3 Compare February 1, 2025 20:42
@camchenry camchenry force-pushed the 02-01-perf_linter_use_parallel_iterator_directly_instead_of_iter_and_parallel_bridge branch from 62585b3 to c5ca0ba Compare February 1, 2025 20:43
Copy link

codspeed-hq bot commented Feb 1, 2025

CodSpeed Performance Report

Merging #8831 will not alter performance

Comparing 02-01-perf_linter_use_parallel_iterator_directly_instead_of_iter_and_parallel_bridge (c5ca0ba) with main (0568210)

Summary

✅ 33 untouched benchmarks

@camchenry
Copy link
Contributor Author

This didn't end up making any appreciable difference when benchmarking, so closing

@camchenry camchenry closed this Feb 1, 2025
@Boshen Boshen reopened this Feb 2, 2025
@Boshen
Copy link
Member

Boshen commented Feb 2, 2025

Let me take a closer look. Codspeed does not bench on multiple files.

@camchenry
Copy link
Contributor Author

@Boshen I did benchmark it locally on vscode but didn't see any difference, sometimes it was slower, sometimes it was faster. Statistically it was all just noise. Theoretically it seems like this should be better based on the docs so maybe it's still valid to merge

@Boshen Boshen marked this pull request as ready for review February 2, 2025 06:26
@Boshen Boshen merged commit 8a4988d into main Feb 2, 2025
27 checks passed
@Boshen Boshen deleted the 02-01-perf_linter_use_parallel_iterator_directly_instead_of_iter_and_parallel_bridge branch February 2, 2025 06:27
Boshen added a commit that referenced this pull request Feb 6, 2025
## [0.15.10] - 2025-02-06

### Features

- d6d80f7 linter: Add suggestion fixer for `eslint/no-iterator` (#8894)
(dalaoshu)
- 7e8568b linter: Junit reporter (#8756) (Tapan Prakash)
- f4662a9 oxc_language_server: Implement `oxc.fixAll` workspace command
(#8858) (Marek Vospel)

### Bug Fixes

- baf3e4e linter: Correctly replace rule severity with duplicate rule
name configurations (#8840) (dalaoshu)

### Performance

- 8a4988d linter: Use parallel iterator directly instead of iter and
parallel bridge (#8831) (Cam McHenry)

### Refactor

- bb9d763 linter: Remove usage of `url` crate (#8833) (camchenry)
- 4fcf719 linter: Replace MIME guessing with extension check (#8832)
(camchenry)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter 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