Skip to content

Comments

perf(transformer): SparseStack always keep minimum 1 entry#5962

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

perf(transformer): SparseStack always keep minimum 1 entry#5962
graphite-app[bot] merged 1 commit intomainfrom
09-22-perf_transformer_sparsestack_always_keep_minimum_1_entry

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Sep 22, 2024

Optimize SparseStack (which was introduced in #5940). Initialize it with a single entry, and ensure the stack is never emptied. This makes take, get_or_init and get_mut_or_init methods infallible, since there is always an entry on the stack to read.

@graphite-app
Copy link
Contributor

graphite-app bot commented Sep 22, 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.

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 22, 2024

CodSpeed Performance Report

Merging #5962 will not alter performance

Comparing 09-22-perf_transformer_sparsestack_always_keep_minimum_1_entry (7b90d79) with main (97a2c41)

Summary

✅ 29 untouched benchmarks

@overlookmotel overlookmotel force-pushed the 09-22-perf_transformer_sparsestack_always_keep_minimum_1_entry branch from 9709e1e to 0fb4067 Compare September 22, 2024 00:49
@overlookmotel overlookmotel marked this pull request as ready for review September 22, 2024 00:50
@overlookmotel
Copy link
Member Author

overlookmotel commented Sep 22, 2024

This series of PRs for SparseStack gains about +1% on transformer benchmarks in total. Not as much as I'd hoped, but still 1% here and 1% there adds up. And I think SparseStack is a useful abstraction, as we're using stacks which fit this pattern a lot in the transformer.

I'll follow up with another PR later on to use 1 stack shared between all these transforms, rather than each maintaining their own stack.

@overlookmotel overlookmotel force-pushed the 09-22-perf_transformer_sparsestack_always_keep_minimum_1_entry branch 2 times, most recently from 4313323 to f5c00b9 Compare September 22, 2024 10:48
@Dunqing Dunqing 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, 3:52 AM 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, 3:52 AM EDT: Dunqing added this pull request to the Graphite merge queue.
  • Sep 23, 4:04 AM EDT: Dunqing merged this pull request with the Graphite merge queue.

Optimize `SparseStack` (which was introduced in #5940). Initialize it with a single entry, and ensure the stack is never emptied. This makes `take`, `get_or_init` and `get_mut_or_init` methods infallible, since there is always an entry on the stack to read.
@Dunqing Dunqing force-pushed the 09-22-perf_transformer_logical_assignment_operator_transform_use_sparsestack_ branch from b5ab0b0 to 28fe80a Compare September 23, 2024 07:54
@Dunqing Dunqing force-pushed the 09-22-perf_transformer_sparsestack_always_keep_minimum_1_entry branch from f5c00b9 to 7b90d79 Compare September 23, 2024 07:55
Base automatically changed from 09-22-perf_transformer_logical_assignment_operator_transform_use_sparsestack_ to main September 23, 2024 08:03
@graphite-app graphite-app bot merged commit 7b90d79 into main Sep 23, 2024
@graphite-app graphite-app bot deleted the 09-22-perf_transformer_sparsestack_always_keep_minimum_1_entry branch September 23, 2024 08:04
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>
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-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants