Skip to content

feat: enable CPU throttling with --cpuThrottle flag - #30

Merged
Rowno merged 2 commits into
Rowno:masterfrom
goooseman:feat/throttling
Jul 31, 2022
Merged

feat: enable CPU throttling with --cpuThrottle flag#30
Rowno merged 2 commits into
Rowno:masterfrom
goooseman:feat/throttling

Conversation

@goooseman

Copy link
Copy Markdown
Contributor

Feature description

To get more accurate results for comparison it is a good idea to run Chrome with 4x-8x CPU throttling. This way the results will be less influenced by other processes running on the same machine.

This PR adds a special --cpuThrottle which is 1 by default not to change the default behavior. User can set it to any number value (e.g. --cpuThrottle 4) to enable CPU throttling.

PR description

  • New CLI flag is added
  • New options argument is added to ReactBenchmark
  • New test is added to E2E tests (test/cli.js) to make sure this option works correctly
  • Docs and typescript definitions are updated

P.S. I was using this project for my personal needs and I have much more changes to contribute in the future, such as:

  • Testing performance not until component is rendered, but until onReady callback is called (useful for async tests)
  • Measuring not only CPU, but also RAM consumption
  • Measuring network consumption
  • Adding configurable delay between tests to give Chrome some time to cooldown
  • Optional rendering to DOM instead of in-memory rendering which is used right now

@goooseman

Copy link
Copy Markdown
ContributorAuthor

@Rowno ping ping 🏓

Comment threadtest/cli.js Outdated
Comment threadlib/index.js
@goooseman

Copy link
Copy Markdown
ContributorAuthor

@Rowno lets merge it? 😉

@Rowno
Rowno merged commit ef2eb8d into Rowno:masterJul 31, 2022
@Rowno

Copy link
Copy Markdown
Owner

It looks like your new unit test is failing consistently in CI on master.

@goooseman

Copy link
Copy Markdown
ContributorAuthor

@Rowno checking...

@goooseman

Copy link
Copy Markdown
ContributorAuthor

@Rowno I see it is running right now, waiting for the tests to fail

BTW, don't you think we should enabled CI also on PR, not only in master branch? That can potentially save from such situations to happen.

I can help you with that, if you want.

@goooseman

Copy link
Copy Markdown
ContributorAuthor

@Rowno I see where the test fail, maybe the threshold is too low, I will update the test in a separate PR

@Rowno

Copy link
Copy Markdown
Owner

Odd, CI should be running for pul requests 🤔

@goooseman

Copy link
Copy Markdown
ContributorAuthor

@Rowno creating a PR with test fix + CI config right now

Sign up for freeto 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.

2 participants

@goooseman@Rowno