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(lexer): use bitshifting when parsing known integers #3296

Merged
merged 7 commits into from
May 17, 2024

Conversation

DonIsaac
Copy link
Collaborator

What This PR Does

  • perf(lexer): use bit shifting when parsing hex, octal, and binary integers instead of mul_add-ing on f64s. Check out the difference in assembly generated here
  • perf(lexer): skip redundant utf8 check when parsing BigInts
  • refactor(lexer): remove unsafe usage (as per @overlookmotel's request here)
  • test(lexer): add numeric parsing unit tests

I don't expect this PR to have a large performance improvement, since the most common case (Kind::Decimal) is not affected. We could do this, however, by splitting Kind::Decimal into Kind::DecimalFloat and Kind::DecimalInt when the lexer encounters a .

@DonIsaac DonIsaac added A-parser Area - Parser C-performance Category - Solution not expected to change functional behavior, only performance labels May 15, 2024
@DonIsaac DonIsaac self-assigned this May 15, 2024
Copy link

graphite-app bot commented May 15, 2024

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

Add the label “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.

Copy link

codspeed-hq bot commented May 15, 2024

CodSpeed Performance Report

Merging #3296 will not alter performance

Comparing don/perf/lexer/shift-when-parsing-ints (61757a1) with main (712ee0d)

Summary

✅ 27 untouched benchmarks

crates/oxc_parser/Cargo.toml Show resolved Hide resolved
crates/oxc_parser/src/lexer/number.rs Outdated Show resolved Hide resolved
@Boshen Boshen requested a review from overlookmotel May 16, 2024 03:00
@Boshen
Copy link
Member

Boshen commented May 16, 2024

use bit shifting

Can you comment why it's better on the spot for future contributors.

@overlookmotel
Copy link
Collaborator

Just to say, the bitshift algorithm is brilliant! Very smart.

Interesting that compiler unrolls the loop. Looks like it unrolls in batches of 4 characters, which should be good for this use case. If it had used batches of 8 chars for octals, that'd likely be a de-opt because JS rarely contains numbers which are as large as 8 digits (decimal or octal).

@DonIsaac
Copy link
Collaborator Author

use bit shifting

Can you comment why it's better on the spot for future contributors.

In this PR discussion, or in the code?

@overlookmotel
Copy link
Collaborator

overlookmotel commented May 16, 2024

use bit shifting

Can you comment why it's better on the spot for future contributors.

In this PR discussion, or in the code?

I think by "on the spot", Boshen means in the code in the place where it happens.

@Boshen
Copy link
Member

Boshen commented May 16, 2024

Yes, in the code.

@DonIsaac DonIsaac requested a review from Boshen May 17, 2024 00:41
Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant work! Thank you!

@Boshen Boshen merged commit 27030b9 into main May 17, 2024
28 checks passed
@Boshen Boshen deleted the don/perf/lexer/shift-when-parsing-ints branch May 17, 2024 01:39
Boshen pushed a commit that referenced this pull request May 21, 2024
Follow-on after #3296.

Make parsing binary/octal/hex numeric literals a little more efficient.

These changes all rely on that we know more than the compiler does -
that strings passed to these `parse_*` functions can only contain a
certain set of characters.
Brooooooklyn pushed a commit to toeverything/AFFiNE that referenced this pull request Jun 11, 2024
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [oxlint](https://oxc.rs) ([source](https://togithub.com/oxc-project/oxc/tree/HEAD/npm/oxlint)) | [`0.3.5` -> `0.4.3`](https://renovatebot.com/diffs/npm/oxlint/0.3.5/0.4.3) | [![age](https://developer.mend.io/api/mc/badges/age/npm/oxlint/0.4.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/oxlint/0.4.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/oxlint/0.3.5/0.4.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/oxlint/0.3.5/0.4.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>oxc-project/oxc (oxlint)</summary>

### [`v0.4.3`](https://togithub.com/oxc-project/oxc/releases/tag/oxlint_v0.4.3): oxlint v0.4.3

[Compare Source](https://togithub.com/oxc-project/oxc/compare/031bbe6dc4b0707b324bee7c9f49543a4c8bf212...4b9a36512ec5186babdf726e604336bfc6838f6c)

#### \[0.4.3] - 2024-06-07

##### Features

-   [`1fb9d23`](https://togithub.com/oxc-project/oxc/commit/1fb9d23) linter: Add fixer for no-useless-fallback-in-spread rule ([#&#8203;3544](https://togithub.com/oxc-project/oxc/issues/3544)) (Don Isaac)
-   [`6506d08`](https://togithub.com/oxc-project/oxc/commit/6506d08) linter: Add fixer for no-single-promise-in-promise-methods ([#&#8203;3531](https://togithub.com/oxc-project/oxc/issues/3531)) (Don Isaac)
-   [`daf559f`](https://togithub.com/oxc-project/oxc/commit/daf559f) linter: Eslint-plugin-jest/no-large-snapshot ([#&#8203;3436](https://togithub.com/oxc-project/oxc/issues/3436)) (cinchen)
-   [`4c17bc6`](https://togithub.com/oxc-project/oxc/commit/4c17bc6) linter: Eslint/no-constructor-return ([#&#8203;3321](https://togithub.com/oxc-project/oxc/issues/3321)) (谭光志)
-   [`4a075cc`](https://togithub.com/oxc-project/oxc/commit/4a075cc) linter/jsdoc: Implement require-param rule ([#&#8203;3554](https://togithub.com/oxc-project/oxc/issues/3554)) (Yuji Sugiura)
-   [`747500a`](https://togithub.com/oxc-project/oxc/commit/747500a) linter/jsdoc: Implement require-returns-type rule ([#&#8203;3458](https://togithub.com/oxc-project/oxc/issues/3458)) (Yuji Sugiura)
-   [`6b39654`](https://togithub.com/oxc-project/oxc/commit/6b39654) linter/tree-shaking: Support options ([#&#8203;3504](https://togithub.com/oxc-project/oxc/issues/3504)) (Wang Wenzhe)

##### Bug Fixes

-   [`b188778`](https://togithub.com/oxc-project/oxc/commit/b188778) linter/eslint: Fix `require-await` false positives in `ForOfStatement`. ([#&#8203;3457](https://togithub.com/oxc-project/oxc/issues/3457)) (rzvxa)
-   [`350cd91`](https://togithub.com/oxc-project/oxc/commit/350cd91) parser: Should parser error when function declaration has no name ([#&#8203;3461](https://togithub.com/oxc-project/oxc/issues/3461)) (Dunqing)

### [`v0.4.2`](https://togithub.com/oxc-project/oxc/releases/tag/oxlint_v0.4.2): oxlint v0.4.2

[Compare Source](https://togithub.com/oxc-project/oxc/compare/d971c9cd0b7da94896179ebd5a8b95b62d05a1c1...031bbe6dc4b0707b324bee7c9f49543a4c8bf212)

#### What's Changed

##### Linter

-   `require-await` by [@&#8203;todor-a](https://togithub.com/todor-a) in [oxc-project/oxc#3406
-   `eslint-plugin-unicorn/no-magic-array` flat depth by [@&#8203;camc314](https://togithub.com/camc314) in [oxc-project/oxc#3411
-   `@typescript-eslint/consistent-indexed-object-style` by [@&#8203;todor-a](https://togithub.com/todor-a) in [oxc-project/oxc#3126
-   `oxc/no-async-await` rule by [@&#8203;mysteryven](https://togithub.com/mysteryven) in [oxc-project/oxc#3438
-   `oxc/no-const-enum` rule by [@&#8203;mysteryven](https://togithub.com/mysteryven) in [oxc-project/oxc#3435
-   `oxc/no-rest-spread-properties` rule by [@&#8203;mysteryven](https://togithub.com/mysteryven) in [oxc-project/oxc#3432
-   `eslint-plugin-jest/prefer-hooks-on-top` by [@&#8203;eryue0220](https://togithub.com/eryue0220) in [oxc-project/oxc#3437
-   `eslint-plugin-jest/require-top-level-describe` by [@&#8203;eryue0220](https://togithub.com/eryue0220) in [oxc-project/oxc#3439
-   `no-div-regex` by [@&#8203;jelly](https://togithub.com/jelly) in [oxc-project/oxc#3442
-   `no-useless-concat` by [@&#8203;jelly](https://togithub.com/jelly) in [oxc-project/oxc#3363
-   prefer-string-starts-ends-with: accept more valid regex by [@&#8203;magic-akari](https://togithub.com/magic-akari) in [oxc-project/oxc#3408

##### Parser

-   fix(parser): parse const extends in arrow functions correctly by [@&#8203;Dunqing](https://togithub.com/Dunqing) in [oxc-project/oxc#3450

#### New Contributors

-   [@&#8203;Bassadin](https://togithub.com/Bassadin) made their first contribution in [oxc-project/oxc#3433

**Full Changelog**: oxc-project/oxc@oxlint_v0.4.1...oxlint_v0.4.2

### [`v0.4.1`](https://togithub.com/oxc-project/oxc/releases/tag/oxlint_v0.4.1): oxlint v0.4.1

[Compare Source](https://togithub.com/oxc-project/oxc/compare/e5337f00b1c7b49cb09d28b860e112d4ebad3233...d971c9cd0b7da94896179ebd5a8b95b62d05a1c1)

#### What's Changed

-   fix(parser): fix lexer error while parsing parenthesized arrow expressions by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3400

The previous version refactored some parsing code around arrow expressions, which failed to parse the following snippet correctly

```javascript
(/\./.exec())
```

**Full Changelog**: oxc-project/oxc@oxlint_v0.4.0...oxlint_v0.4.1

### [`v0.4.0`](https://togithub.com/oxc-project/oxc/releases/tag/oxlint_v0.4.0): oxlint v0.4.0

[Compare Source](https://togithub.com/oxc-project/oxc/compare/754d9f4c98aab052cf6b2756f7af12557042708d...e5337f00b1c7b49cb09d28b860e112d4ebad3233)

#### Potential Breaking Changes

##### enforce rule severity from the cli and configuration file by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3337

The `--deny` or `-D` flag in the CLI, and the `"error"` severity setting in the configuration file will now set linter diagnostics to be an "error" and exit the program with exit code 1.

Previously, these flags had no effect, and all linter diagnostics were reported as warnings.

This means in CI, `oxlint --deny-warnings` is no longer needed for exit code 1 if `oxlint -D correctness` is set.

To restore the previous "report as warning" behaviour, the `--warn` or `-W` flag is added to the CLI, and the `"warn"` severity in the configuration file will take into effect.

##### merge deepscan rules into oxc rules by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3327

`deepscan` rules are now "oxc" rules, because there is no "deepscan" plugin in the eslint ecosystem and this caused some confusion.

#### Ecosystem CI

We added the [`Oxlint Ecosystem CI`](https://togithub.com/oxc-project/oxlint-ecosystem-ci/actions/workflows/ecosystem-ci.yml) to maximize ecosystem compatibility, reduce churn, and minimize break-after-release

#### New Features

-   `--disable-oxc-plugin` by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3328
-   `--disable`-react/unicorn/typescript-`plugin` by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3305

The default rule set enables some plugins by default, the following CLI arguments are added for disabling them:

-   `--disable-react-plugin`
-   `--disable-unicorn-plugin`
-   `--disable-oxc-plugin`
-   `--disable-typescript-plugin`

#### New Rules

Two notable new rules that are under experiment but worth a try:

##### No Barrel File

`oxlint --import-plugin -D no-barrel-file`

Loading a lot of modules is slow for runtimes and bundlers.

<img width="600" alt="image" src="https://github.com/oxc-project/oxc/assets/1430279/964d8615-a5c2-4934-a1e4-a6f8b023e199">

To change the threshhold:

`oxlint -c oxlintrc.json --import-plugin -D no-barrel-file`

```json
{
  "rules": {
    "oxc/no-barrel-file": ["error", {
      "threshold": 10
    }]
  }
}
```

See [Speeding up the JavaScript ecosystem - The barrel file debacle](https://marvinh.dev/blog/speeding-up-javascript-ecosystem-part-7) for background reading.

##### Rule of Hooks

`oxlint -D rules-of-hooks`

Enforce the React Rules of Hooks.

-   `eslint-plugin-jest/no-duplicate-hooks` by [@&#8203;eryue0220](https://togithub.com/eryue0220) in [oxc-project/oxc#3358
-   `default-case` rule by [@&#8203;jelly](https://togithub.com/jelly) in [oxc-project/oxc#3379
-   `no-new` by [@&#8203;jelly](https://togithub.com/jelly) in [oxc-project/oxc#3368
-   `prefer-exponentiation-operator` by [@&#8203;jelly](https://togithub.com/jelly) in [oxc-project/oxc#3365
-   `symbol-description` by [@&#8203;jelly](https://togithub.com/jelly) in [oxc-project/oxc#3364
-   `jsdoc/require-returns-description` by [@&#8203;leaysgur](https://togithub.com/leaysgur) in [oxc-project/oxc#3397

#### Bug Fixes

-   fix(linter): avoid infinite loop in `jest/expect-expect` by [@&#8203;mysteryven](https://togithub.com/mysteryven) in [oxc-project/oxc#3332
-   fix(linter): avoid infinite loop when traverse ancestors in `jest/no_conditional_expect` by [@&#8203;mysteryven](https://togithub.com/mysteryven) in [oxc-project/oxc#3330
-   fix(linter): fix panic in jest/expect-expect by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3324
-   fix(linter/jsx-no-undef): check for globals when an identifier is undefined by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3331
-   fix(linter/next): false positives for non-custom font link by [@&#8203;Dunqing](https://togithub.com/Dunqing) in [oxc-project/oxc#3383

#### Performance Improvements

-   perf(lexer): use bitshifting when parsing known integers by [@&#8203;DonIsaac](https://togithub.com/DonIsaac) in [oxc-project/oxc#3296
-   perf(linter): use `usize` for `RuleEnum` hash by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3336
-   perf(parser): more efficient number parsing by [@&#8203;overlookmotel](https://togithub.com/overlookmotel) in [oxc-project/oxc#3342
-   perf(parser): use `FxHashSet` for `not_parenthesized_arrow` by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3344
-   perf(parser): improve `parse_simple_arrow_function_expression` by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3349
-   perf(parser): improve expression parsing by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3352
-   perf(parser): improve is_parenthesized_arrow_function_expression by [@&#8203;Boshen](https://togithub.com/Boshen) in [oxc-project/oxc#3343

#### What's coming next

-   Published JSON schema for configuration file
-   More features to the configuration file
-   Automatically synced and never outdated documentation website pages
    -   [Command-line Interface](https://oxc-project.github.io/docs/guide/usage/linter/cli.html)
    -   [Configuration File](https://oxc-project.github.io/docs/guide/usage/linter/config.html)
    -   [Rules Page](https://oxc-project.github.io/docs/guide/usage/linter/rules.html)

**Full Changelog**: oxc-project/oxc@oxlint_v0.3.5...oxlint_v0.4.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/toeverything/AFFiNE).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4zOTMuMCIsInRhcmdldEJyYW5jaCI6ImNhbmFyeSIsImxhYmVscyI6WyJkZXBlbmRlbmNpZXMiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser Area - Parser 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.

None yet

3 participants