Skip to content

Comments

perf(linter, prettier, diagnostics): use FxHashMap instead of std::collections::HashMap#5993

Merged
graphite-app[bot] merged 1 commit intomainfrom
09-23-perf_linter_prettier_diagnostics_use_fxhashmap_instead_of_std_collections_hashmap_
Sep 23, 2024
Merged

perf(linter, prettier, diagnostics): use FxHashMap instead of std::collections::HashMap#5993
graphite-app[bot] merged 1 commit intomainfrom
09-23-perf_linter_prettier_diagnostics_use_fxhashmap_instead_of_std_collections_hashmap_

Conversation

@camchenry
Copy link
Member

@camchenry camchenry commented Sep 23, 2024

Using FxHashMap is faster than HashMap in many cases, especially for hashing-heavy workloads. This change improves the performance of the linter, prettier, and diagnostics crates by using FxHashMap instead of std::collections::HashMap.

@graphite-app
Copy link
Contributor

graphite-app bot commented Sep 23, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added A-linter Area - Linter A-semantic Area - Semantic A-prettier labels Sep 23, 2024
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 23, 2024

CodSpeed Performance Report

Merging #5993 will not alter performance

Comparing 09-23-perf_linter_prettier_diagnostics_use_fxhashmap_instead_of_std_collections_hashmap_ (2b17003) with main (b240b42)

Summary

✅ 29 untouched benchmarks

@camchenry
Copy link
Member Author

The -1% changes appear to be noise related to ModuleRecord. However, the +1% in transformer appears to be genuine:
image

The +1% in linter seems uncertain, but doesn't look like it hurts performance at all. I think this is worth it, just for the +1% in transformer.

@camchenry camchenry marked this pull request as ready for review September 23, 2024 15:11
@Boshen
Copy link
Member

Boshen commented Sep 23, 2024

Oh my! How did these go under the radar? Would you like to try banning these in

disallowed-methods = [
?

@camchenry
Copy link
Member Author

@Boshen yeah, was going to ask about that! I can do that

@camchenry camchenry force-pushed the 09-23-perf_linter_prettier_diagnostics_use_fxhashmap_instead_of_std_collections_hashmap_ branch from d457e63 to 283a97a Compare September 23, 2024 15:20
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Sep 23, 2024
@graphite-app
Copy link
Contributor

graphite-app bot commented Sep 23, 2024

Merge activity

  • Sep 23, 12:25 PM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Sep 23, 12:25 PM EDT: Boshen added this pull request to the Graphite merge queue.
  • Sep 23, 12:34 PM EDT: Boshen merged this pull request with the Graphite merge queue.

…:collections::HashMap` (#5993)

Using `FxHashMap` is faster than `HashMap` in many cases, especially for hashing-heavy workloads. This change improves the performance of the linter, prettier, and diagnostics crates by using `FxHashMap` instead of `std::collections::HashMap`.
@Boshen Boshen force-pushed the 09-23-perf_linter_prettier_diagnostics_use_fxhashmap_instead_of_std_collections_hashmap_ branch from 283a97a to 2b17003 Compare September 23, 2024 16:29
@graphite-app graphite-app bot merged commit 2b17003 into main Sep 23, 2024
@graphite-app graphite-app bot deleted the 09-23-perf_linter_prettier_diagnostics_use_fxhashmap_instead_of_std_collections_hashmap_ branch September 23, 2024 16:34
Boshen added a commit that referenced this pull request Sep 24, 2024
## [0.30.1] - 2024-09-24

### Features

- 5c323a2 minifier: Loop compressor passes (#6013) (Boshen)

### Bug Fixes

- 9ca202a codegen: Preserve newlines between comments (#6014) (Boshen)
- 4a99372 codegen: Print jsdoc comments for `TSEnumMember`s (#6007)
(camc314)
- 97a2c41 isolated-declarations: False positive for class private getter
with non-inferrable return type (#5987) (michaelm)

### Performance

- 2b17003 linter, prettier, diagnostics: Use `FxHashMap` instead of
`std::collections::HashMap` (#5993) (camchenry)
- 7b90d79 transformer: `SparseStack` always keep minimum 1 entry (#5962)
(overlookmotel)
- 28fe80a transformer: Logical assignment operator transform use
`SparseStack` (#5960) (overlookmotel)
- 9f7d4b7 transformer: Exponentiation operator transform use
`SparseStack` (#5959) (overlookmotel)
- 5dc0154 transformer: Nullish coalescing operator transform use
`SparseStack` (#5942) (overlookmotel)
- 618e89e transformer: Arrow function transform: reduce stack memory
usage (#5940) (overlookmotel)

### Documentation

- 5a0d17c ast: Document more AST nodes (#6000) (DonIsaac)
- 18371dd oxc: Include feature-guarded modules in docs.rs (#6012)
(DonIsaac)
- 1abfe8f semantic: Document `SymbolTable` (#5998) (DonIsaac)
- f5eee72 semantic: Correct docs for `Reference` (#5992) (overlookmotel)
- 860f108 transformer: Add to arrow functions transform docs (#5989)
(overlookmotel)

### Refactor

- 0a2f687 minifier: Move dce conditional expression to `RemoveDeadCode`
(#5971) (Boshen)
- f02bf51 transformer: Arrow function transform: remove unnecessary
assertion (#6002) (overlookmotel)

---------

Co-authored-by: Boshen <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@oxc-bot oxc-bot mentioned this pull request Sep 24, 2024
Boshen added a commit that referenced this pull request Sep 24, 2024
## [0.9.8] - 2024-09-24

### Bug Fixes

- e3c8a12 linter: Fix panic in sort-keys (#6017) (Boshen)
- 4771492 linter: Fix `import/no_cycle` with `ignoreTypes` (#5995)
(Boshen)

### Performance

- 5ae3f36 linter: `no-fallthrough`: Use string matching instead of Regex
for default comment pattern (#6008) (camchenry)
- 2b17003 linter, prettier, diagnostics: Use `FxHashMap` instead of
`std::collections::HashMap` (#5993) (camchenry)

---------

Co-authored-by: Boshen <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0-merge Merge with Graphite Merge Queue A-linter Area - Linter A-semantic Area - Semantic

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants