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

fix(bench): Result interpretation problems #5798

Merged
merged 7 commits into from Dec 14, 2022

Conversation

Beanow
Copy link
Member

@Beanow Beanow commented Dec 9, 2022

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

The main reason the benchmarks broke / went weird, were because of ubuntu-latest changing from 20.04 to 22.04.
This PR fixes the interpretation of results. But doesn't address the performance regression in #5800.

It's a newer syscall interface and gets counted seperately in strace results.
The column offsets didn't match up when usecs/call is included in the totals.
On my local machine, it used an NL locale changing the strace numbers to use commas.
Adding this env var avoids that problem.
@Beanow
Copy link
Member Author

Beanow commented Dec 9, 2022

This don't yet fix #5800

The ubuntu update also caused issues with the strace output being different.
These patches account for the new format, but don't address the performance slowdown.

@Beanow Beanow marked this pull request as ready for review December 9, 2022 18:06
@Beanow Beanow requested review from a team as code owners December 9, 2022 18:06
@Beanow
Copy link
Member Author

Beanow commented Dec 9, 2022

The benchmark result can be seen here:
https://github.com/Beanow/tauri/actions/runs/3658215916/jobs/6182717942

Restoring the syscalls and thread count to expected range.

 {
  "created_at": "2022-12-09T14:37:49Z",
  "sha1": "0a9d99551dbdc49ba5945ec896804b7870dc853f",
  "exec_time": {
    "tauri_3mb_transfer": {
      "stddev": 4.31788653464092,
      "system": 0.10376614,
      "min": 13.783955114655,
      "user": 0.269474715,
      "max": 25.774074237655,
      "mean": 23.755779372855
    },
    "tauri_hello_world": {
      "max": 25.362370100655,
      "system": 0.026286739999999996,
      "user": 0.068088815,
      "min": 18.948130902655,
      "mean": 24.050229535955,
      "stddev": 2.6422230777400255
    },
    "tauri_cpu_intensive": {
      "min": 5.8631193956550005,
      "mean": 24.935138477455,
      "stddev": 10.047903412326082,
      "max": 29.713699274655,
      "system": 0.04284094,
      "user": 0.102933015
    }
  },
  "binary_size": {
    "tauri_3mb_transfer": 1863568,
    "wry_rlib": 705990,
    "tauri_cpu_intensive": 1863472,
    "tauri_hello_world": 1863464
  },
  "max_memory": {
    "tauri_hello_world": 277872640,
    "tauri_cpu_intensive": 407896064,
    "tauri_3mb_transfer": 512753664
  },
  "thread_count": {
    "tauri_cpu_intensive": 53,
    "tauri_hello_world": 48,
    "tauri_3mb_transfer": 49
  },
  "syscall_count": {
    "tauri_cpu_intensive": 32445,
    "tauri_hello_world": 25991,
    "tauri_3mb_transfer": 26341
  },
  "cargo_deps": {
    "macOS": 210,
    "Windows": 212,
    "Linux": 230
  }
}

@lucasfernog lucasfernog merged commit f7a080a into tauri-apps:dev Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants