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

feat: add --ram option to measure RAM #32

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

goooseman
Copy link
Contributor

Adds a special --ram flag and isRamMeasured flag to ReactBenchmark.

If passed RAM measurement is enabled. In this case, 2 metrics are being recorded between the runs:

  • Heap size (JSHeapUsedSize) represents how much RAM was consumed at the end of a test iteration.
  • Object.prototype represents how many objects were created in RAM at the end of a test iteration. Why is it interesting? JS engine sometimes optimizes a code in different ways which in turn changes its memory footprint. source / look at the "Counting all the objects" section

P.S. @Rowno this PR should be merged after #30 to avoid extra conflicts

@goooseman goooseman force-pushed the feat/ram-consumption branch from 5e20d59 to 7f890e2 Compare August 3, 2022 11:40
@goooseman
Copy link
Contributor Author

@Rowno ready for review

@goooseman
Copy link
Contributor Author

@Rowno In the end something you was worried about has happened: we do have a flaky test.

And it is flaky on CI only, which makes it completely difficult to debug.

As of now I do have 0 ideas of why is it happening: even though CI is much less powerful than my local machine, but still CPU threshold should make it even 4x times slower, so the tests should always pass.

I will try to research this a little bit.

@goooseman
Copy link
Contributor Author

@Rowno From this thread I see there is a problem in CPU throttling of Chrome running inside docker.

I believe it is better to remove this failing test: it does not provide as much as quality, as number of problems it creates.

@Rowno
Copy link
Owner

Rowno commented Aug 5, 2022

Yeah, I would remove the test.

@goooseman goooseman force-pushed the feat/ram-consumption branch from 76d6b68 to 24e2293 Compare August 5, 2022 08:55
@goooseman
Copy link
Contributor Author

@Rowno ahaha, I've also deleted it in this PR.

Updated PR, ready for review

@goooseman
Copy link
Contributor Author

@Rowno WDYT about this change?

@goooseman
Copy link
Contributor Author

@Rowno ping

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.

2 participants