Run LSP benchmarks in CI #5546

sdankel · 2024-02-02T21:37:44Z

Description

Runs benchmarks in CI with criterion-compare-prs so the comparison shows as a comment on the PR.

The extra config in Cargo.toml is because of this issue: bheisler/criterion.rs#275. It needs to be merged to master first (#5547) for the benchmark comparison to work.

Checklist

I have linked to any relevant issues.
I have commented my code, particularly in hard-to-understand areas.
I have updated the documentation where relevant (API docs, the reference, and the Sway book).
I have added tests that prove my fix is effective or that my feature works.
I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
I have requested a review from the relevant team or maintainers.

The extra config in Cargo.toml is because of this issue: bheisler/criterion.rs#275 Once merged, this PR should be working: #5546

## Description The extra config in Cargo.toml is because of this issue: bheisler/criterion.rs#275 Once merged, this PR should be working: #5546 This doesn't affect running benchmarks locally. ## Checklist - [ ] I have linked to any relevant issues. - [ ] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers.

github-actions · 2024-02-03T02:48:26Z

Benchmark for `dbb86a5`

Click to view benchmark

Test	Base	PR	%
code_action	5.3±0.02ms	5.3±0.03ms	0.00%
code_lens	321.6±6.93ns	324.3±5.91ns	+0.84%
compile	2.9±0.05s	2.9±0.04s	0.00%
completion	4.9±0.10ms	4.8±0.02ms	-2.04%
did_change_with_caching	2.9±0.03s	2.9±0.02s	0.00%
document_symbol	991.5±17.84µs	966.8±15.17µs	-2.49%
format	74.4±0.69ms	74.0±1.11ms	-0.54%
goto_definition	360.1±6.65µs	353.4±5.61µs	-1.86%
highlight	9.0±0.02ms	9.1±0.09ms	+1.11%
hover	528.9±11.38µs	534.5±25.07µs	+1.06%
idents_at_position	124.3±0.39µs	122.6±0.81µs	-1.37%
inlay_hints	671.5±10.24µs	663.8±26.21µs	-1.15%
on_enter	489.5±9.93ns	493.0±16.26ns	+0.72%
parent_decl_at_position	3.7±0.03ms	3.7±0.02ms	0.00%
prepare_rename	356.0±5.52µs	358.5±10.11µs	+0.70%
rename	9.4±0.19ms	9.4±0.02ms	0.00%
semantic_tokens	1057.6±21.00µs	1041.1±20.44µs	-1.56%
token_at_position	353.5±2.17µs	350.5±2.06µs	-0.85%
tokens_at_position	3.7±0.02ms	3.7±0.17ms	0.00%
tokens_for_file	412.8±0.92µs	411.7±2.63µs	-0.27%
traverse	39.0±1.02ms	39.2±0.76ms	+0.51%

github-actions · 2024-02-04T05:38:16Z

Benchmark for `ca37ade`

Click to view benchmark

Test	Base	PR	%
code_action	5.3±0.10ms	5.3±0.12ms	0.00%
code_lens	331.8±19.78ns	324.3±10.21ns	-2.26%
compile	3.0±0.06s	2.9±0.02s	-3.33%
completion	4.9±0.12ms	4.9±0.10ms	0.00%
did_change_with_caching	2.8±0.03s	3.0±0.04s	+7.14%
document_symbol	968.3±19.65µs	1038.4±14.61µs	+7.24%
format	75.1±1.05ms	74.7±1.24ms	-0.53%
goto_definition	359.9±6.13µs	355.3±9.53µs	-1.28%
highlight	9.2±0.14ms	9.1±0.08ms	-1.09%
hover	538.5±6.19µs	525.2±7.86µs	-2.47%
idents_at_position	123.6±1.01µs	123.2±0.86µs	-0.32%
inlay_hints	666.2±23.44µs	670.2±26.59µs	+0.60%
on_enter	494.5±18.47ns	501.4±10.59ns	+1.40%
parent_decl_at_position	3.8±0.06ms	3.7±0.03ms	-2.63%
prepare_rename	358.1±6.05µs	350.9±9.19µs	-2.01%
rename	9.7±0.18ms	9.5±0.18ms	-2.06%
semantic_tokens	1063.7±15.94µs	1055.9±15.16µs	-0.73%
token_at_position	354.4±4.01µs	348.7±2.53µs	-1.61%
tokens_at_position	3.7±0.13ms	3.7±0.05ms	0.00%
tokens_for_file	413.3±2.00µs	409.9±5.71µs	-0.82%
traverse	39.5±0.80ms	39.2±1.27ms	-0.76%

JoshuaBatty · 2024-02-04T23:36:32Z

Nice this will be handy. Just wondering if we should filter this job to run only if code in sway-core of sway-lsp was changed so it doesn't become noisy.

  benchmark-sway-lsp:
    name: Compare sway-lsp benchmarks against master
    runs-on: ubuntu-latest
    if: >-
      contains(toJSON(github.event.pull_request.changed_files), 'sway-lsp/') ||
      contains(toJSON(github.event.pull_request.changed_files), 'sway-core/')
    steps:
      - uses: Swatinem/rust-cache@v2
      - uses: actions/checkout@v3
      - uses: boa-dev/[email protected]
        with:
          cwd: "./sway-lsp"
          branchName: ${{ github.base_ref }}
          token: ${{ secrets.GITHUB_TOKEN }}

sdankel · 2024-02-05T19:06:00Z

Nice this will be handy. Just wondering if we should filter this job to run only if code in sway-core of sway-lsp was changed so it doesn't become noisy.

  benchmark-sway-lsp:
    name: Compare sway-lsp benchmarks against master
    runs-on: ubuntu-latest
    if: >-
      contains(toJSON(github.event.pull_request.changed_files), 'sway-lsp/') ||
      contains(toJSON(github.event.pull_request.changed_files), 'sway-core/')
    steps:
      - uses: Swatinem/rust-cache@v2
      - uses: actions/checkout@v3
      - uses: boa-dev/[email protected]
        with:
          cwd: "./sway-lsp"
          branchName: ${{ github.base_ref }}
          token: ${{ secrets.GITHUB_TOKEN }}

Good call out. I realized that we also only want to run it on pull requests, not releases or pushes to master. I think it's cleanest as it's own workflow. What do you think?

I also added all of the dependencies from sway-lsp's Cargo.toml that are in this repo.

github-actions · 2024-02-05T19:34:35Z

Benchmark for `2c186a1`

Click to view benchmark

Test	Base	PR	%
code_action	5.3±0.13ms	5.3±0.02ms	0.00%
code_lens	283.1±8.52ns	284.6±8.87ns	+0.53%
compile	2.8±0.02s	2.8±0.03s	0.00%
completion	4.9±0.10ms	4.9±0.11ms	0.00%
did_change_with_caching	2.8±0.03s	2.9±0.02s	+3.57%
document_symbol	1043.0±47.49µs	967.8±11.62µs	-7.21%
format	70.8±0.98ms	70.4±1.26ms	-0.56%
goto_definition	355.0±6.48µs	363.5±8.24µs	+2.39%
highlight	9.1±0.18ms	9.2±0.13ms	+1.10%
hover	525.7±6.56µs	523.7±7.24µs	-0.38%
idents_at_position	131.1±0.28µs	131.2±1.25µs	+0.08%
inlay_hints	665.0±15.65µs	662.0±8.76µs	-0.45%
on_enter	484.0±15.14ns	479.6±13.39ns	-0.91%
parent_decl_at_position	3.7±0.03ms	3.8±0.05ms	+2.70%
prepare_rename	356.5±9.23µs	373.1±5.00µs	+4.66%
rename	9.6±0.24ms	9.4±0.17ms	-2.08%
semantic_tokens	1070.3±51.74µs	1029.6±16.97µs	-3.80%
token_at_position	350.2±3.20µs	352.4±2.79µs	+0.63%
tokens_at_position	3.7±0.09ms	3.8±0.02ms	+2.70%
tokens_for_file	410.0±9.93µs	505.5±3.78µs	+23.29%
traverse	39.2±1.13ms	38.8±1.23ms	-1.02%

.github/workflows/bench-lsp.yml

JoshuaBatty

❤️

xunilrj · 2024-02-08T08:07:29Z

How does this chooses the commit to run?

Last comment has Benchmark for https://github.com/FuelLabs/sway/commit/c1daf98ba4e34562a59150a3c654bcb228307463, but there is not commit c1da.

Hovering the link it says [Merge 9de8b0e483a133a3f5cfbace9be20f1432f472b0 into f912881c71174655c…](https://github.com/FuelLabs/sway/commit/c1daf98ba4e34562a59150a3c654bcb228307463), but 9de8b is not the last commit.

sdankel · 2024-02-08T20:12:37Z

How does this chooses the commit to run?

Good catch, I would also expect the commits to be in the PR branch. It looks like the changes in the commit are correct, and the benchmark comparison is correct, so I think it's still worth using this. I've opened an issue: boa-dev/criterion-compare-action#115

.github/workflows/bench-lsp.yml

github-actions · 2024-02-08T21:23:15Z

Benchmark for `4bc8877`

Click to view benchmark

Test	Base	PR	%
code_action	5.4±0.11ms	5.4±0.09ms	0.00%
code_lens	326.9±7.60ns	334.3±11.32ns	+2.26%
compile	3.1±0.05s	3.0±0.05s	-3.23%
completion	4.9±0.09ms	4.9±0.04ms	0.00%
did_change_with_caching	2.9±0.04s	2.9±0.03s	0.00%
document_symbol	950.5±12.69µs	961.0±18.31µs	+1.10%
format	71.3±1.00ms	71.3±1.21ms	0.00%
goto_definition	347.5±6.17µs	351.8±10.51µs	+1.24%
highlight	9.2±0.16ms	9.2±0.12ms	0.00%
hover	534.7±11.02µs	532.3±9.82µs	-0.45%
idents_at_position	121.1±1.41µs	122.0±1.06µs	+0.74%
inlay_hints	659.1±15.02µs	661.1±20.49µs	+0.30%
on_enter	478.9±14.22ns	490.8±22.29ns	+2.48%
parent_decl_at_position	3.7±0.05ms	3.7±0.03ms	0.00%
prepare_rename	359.3±18.14µs	351.9±4.90µs	-2.06%
rename	9.6±0.18ms	9.5±0.13ms	-1.04%
semantic_tokens	1066.2±25.45µs	1047.7±17.00µs	-1.74%
token_at_position	352.1±4.21µs	355.8±4.05µs	+1.05%
tokens_at_position	3.7±0.14ms	3.8±0.07ms	+2.70%
tokens_for_file	408.1±5.41µs	450.4±2.59µs	+10.37%
traverse	39.3±0.96ms	39.5±0.78ms	+0.51%

xunilrj · 2024-02-09T11:20:30Z

I agree with you that it seems to be working by the github action code. Have just approved this PR whilst we wait to see what the action author thinks of the modification.

github-actions · 2024-02-10T04:36:38Z

Benchmark for `bb5d0ad`

Click to view benchmark

Test	Base	PR	%
code_action	5.1±0.13ms	5.1±0.07ms	0.00%
code_lens	301.1±21.31ns	291.1±12.71ns	-3.32%
compile	3.0±0.02s	2.9±0.03s	-3.33%
completion	4.7±0.08ms	4.7±0.06ms	0.00%
did_change_with_caching	2.8±0.01s	2.8±0.03s	0.00%
document_symbol	945.6±7.80µs	953.6±10.11µs	+0.85%
format	89.7±1.04ms	89.7±1.28ms	0.00%
goto_definition	346.7±6.18µs	396.0±4.43µs	+14.22%
highlight	8.7±0.01ms	8.7±0.12ms	0.00%
hover	537.2±6.61µs	561.3±9.59µs	+4.49%
idents_at_position	121.0±0.57µs	122.1±0.75µs	+0.91%
inlay_hints	645.2±16.46µs	652.3±22.75µs	+1.10%
on_enter	490.0±29.41ns	481.1±13.97ns	-1.82%
parent_decl_at_position	3.6±0.04ms	3.6±0.08ms	0.00%
prepare_rename	345.4±4.09µs	356.5±5.92µs	+3.21%
rename	9.1±0.20ms	9.1±0.21ms	0.00%
semantic_tokens	1042.8±8.53µs	1031.1±13.62µs	-1.12%
token_at_position	341.2±2.57µs	353.4±2.53µs	+3.58%
tokens_at_position	3.6±0.03ms	3.6±0.04ms	0.00%
tokens_for_file	402.2±2.64µs	409.8±2.81µs	+1.89%
traverse	38.3±1.44ms	37.9±0.59ms	-1.04%

github-actions · 2024-02-12T22:20:20Z

Benchmark for `52d59c4`

Click to view benchmark

Test	Base	PR	%
code_action	5.3±0.12ms	5.3±0.17ms	0.00%
code_lens	286.5±8.02ns	286.0±7.15ns	-0.17%
compile	3.1±0.03s	3.1±0.03s	0.00%
completion	5.0±0.20ms	5.0±0.19ms	0.00%
did_change_with_caching	3.0±0.03s	3.0±0.01s	0.00%
document_symbol	1025.6±30.01µs	1016.9±36.68µs	-0.85%
format	90.9±1.28ms	90.6±2.75ms	-0.33%
goto_definition	366.2±9.10µs	351.2±6.33µs	-4.10%
highlight	8.9±0.12ms	8.8±0.18ms	-1.12%
hover	542.5±7.62µs	562.8±10.12µs	+3.74%
idents_at_position	122.1±0.48µs	122.0±1.85µs	-0.08%
inlay_hints	661.9±19.61µs	657.7±16.50µs	-0.63%
on_enter	493.8±14.59ns	493.8±20.48ns	0.00%
parent_decl_at_position	3.6±0.05ms	3.6±0.04ms	0.00%
prepare_rename	354.5±6.54µs	358.2±5.07µs	+1.04%
rename	9.6±0.25ms	9.5±0.38ms	-1.04%
semantic_tokens	1058.2±11.40µs	1065.0±27.35µs	+0.64%
token_at_position	356.7±2.83µs	354.3±3.16µs	-0.67%
tokens_at_position	3.6±0.06ms	3.6±0.04ms	0.00%
tokens_for_file	416.6±4.42µs	414.6±4.54µs	-0.48%
traverse	39.3±1.02ms	39.2±0.84ms	-0.25%

sdankel added 6 commits November 27, 2023 17:24

Add gh action to compare benchmarks on PR

b068e76

remove features

5e25eab

version

a7d956b

rust cache

a3ad664

rm toolchain

6953973

fix benchmarks

830aff8

sdankel changed the title ~~Sophie/benchmark action~~ Run LSP benchmarks in CI Feb 2, 2024

sdankel added 2 commits February 2, 2024 13:42

Merge branch 'master' into sophie/benchmark-action

32c331b

cwd

94a6a37

sdankel added a commit that referenced this pull request Feb 2, 2024

Support for criterion-compare-prs gh action

d51931a

The extra config in Cargo.toml is because of this issue: bheisler/criterion.rs#275 Once merged, this PR should be working: #5546

sdankel mentioned this pull request Feb 2, 2024

Support for criterion-compare-prs gh action #5547

Merged

7 tasks

Merge branch 'master' into sophie/benchmark-action

76a6ddb

sdankel marked this pull request as ready for review February 3, 2024 03:20

Merge branch 'master' into sophie/benchmark-action

e9e5114

path filter

34b4fc3

sdankel and others added 4 commits February 5, 2024 11:06

Merge branch 'master' into sophie/benchmark-action

9da64ce

revert comment

94336d6

revert ci.yml

c49e82c

more dependencies

453344c

kayagokalp reviewed Feb 5, 2024

View reviewed changes

.github/workflows/bench-lsp.yml Outdated Show resolved Hide resolved

sdankel and others added 4 commits February 5, 2024 14:09

more deps

9de8b0e

only ready_for_review prs

634855b

Merge branch 'master' into sophie/benchmark-action

264d5f7

cancel in progress

f07ebdf

sdankel requested a review from JoshuaBatty February 6, 2024 21:53

Merge branch 'master' into sophie/benchmark-action

083b2d3

JoshuaBatty assigned sdankel Feb 6, 2024

JoshuaBatty added the ci label Feb 6, 2024

JoshuaBatty previously approved these changes Feb 6, 2024

View reviewed changes

JoshuaBatty requested review from a team February 6, 2024 22:14

sdankel enabled auto-merge (squash) February 6, 2024 23:41

JoshuaBatty and others added 3 commits February 7, 2024 10:47

Merge branch 'master' into sophie/benchmark-action

f14022c

Merge branch 'master' into sophie/benchmark-action

15efa03

Merge branch 'master' into sophie/benchmark-action

927b0f9

sdankel mentioned this pull request Feb 8, 2024

Linked commit has the wrong target boa-dev/criterion-compare-action#115

Open

Merge branch 'master' into sophie/benchmark-action

20f18e8

sdankel commented Feb 8, 2024

View reviewed changes

.github/workflows/bench-lsp.yml Outdated Show resolved Hide resolved

sdankel dismissed JoshuaBatty’s stale review via 410c643 February 8, 2024 21:02

Update .github/workflows/bench-lsp.yml

410c643

sdankel requested a review from JoshuaBatty February 8, 2024 21:48

xunilrj approved these changes Feb 9, 2024

View reviewed changes

Merge branch 'master' into sophie/benchmark-action

87bc330

kayagokalp approved these changes Feb 12, 2024

View reviewed changes

crodas approved these changes Feb 12, 2024

View reviewed changes

Merge branch 'master' into sophie/benchmark-action

69c6ab5

sdankel merged commit e0ae0bd into master Feb 12, 2024
31 checks passed

sdankel deleted the sophie/benchmark-action branch February 12, 2024 22:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run LSP benchmarks in CI #5546

Run LSP benchmarks in CI #5546

sdankel commented Feb 2, 2024 •

edited

Loading

github-actions bot commented Feb 3, 2024

github-actions bot commented Feb 4, 2024

JoshuaBatty commented Feb 4, 2024

sdankel commented Feb 5, 2024 •

edited

Loading

github-actions bot commented Feb 5, 2024

JoshuaBatty left a comment

xunilrj commented Feb 8, 2024

sdankel commented Feb 8, 2024 •

edited

Loading

github-actions bot commented Feb 8, 2024

xunilrj commented Feb 9, 2024

github-actions bot commented Feb 10, 2024

github-actions bot commented Feb 12, 2024

Run LSP benchmarks in CI #5546

Run LSP benchmarks in CI #5546

Conversation

sdankel commented Feb 2, 2024 • edited Loading

Description

Checklist

github-actions bot commented Feb 3, 2024

Benchmark for dbb86a5

github-actions bot commented Feb 4, 2024

Benchmark for ca37ade

JoshuaBatty commented Feb 4, 2024

sdankel commented Feb 5, 2024 • edited Loading

github-actions bot commented Feb 5, 2024

Benchmark for 2c186a1

JoshuaBatty left a comment

Choose a reason for hiding this comment

xunilrj commented Feb 8, 2024

sdankel commented Feb 8, 2024 • edited Loading

github-actions bot commented Feb 8, 2024

Benchmark for 4bc8877

xunilrj commented Feb 9, 2024

github-actions bot commented Feb 10, 2024

Benchmark for bb5d0ad

github-actions bot commented Feb 12, 2024

Benchmark for 52d59c4

sdankel commented Feb 2, 2024 •

edited

Loading

Benchmark for `dbb86a5`

Benchmark for `ca37ade`

sdankel commented Feb 5, 2024 •

edited

Loading

Benchmark for `2c186a1`

sdankel commented Feb 8, 2024 •

edited

Loading

Benchmark for `4bc8877`

Benchmark for `bb5d0ad`

Benchmark for `52d59c4`