-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
test: reduce the allocation size in test-worker-arraybuffer-zerofill #54839
test: reduce the allocation size in test-worker-arraybuffer-zerofill #54839
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
As written elsewhere I really see no reason to refactor this kind of tests to use |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #54839 +/- ##
==========================================
- Coverage 88.07% 88.06% -0.01%
==========================================
Files 651 651
Lines 183396 183396
Branches 35795 35798 +3
==========================================
- Hits 161523 161515 -8
+ Misses 15161 15159 -2
- Partials 6712 6722 +10 |
The stress test looks good. |
This comment was marked as outdated.
This comment was marked as outdated.
4db976a
to
066d836
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Test has been flaky with timeouts in CI. This is possibly due to the repeated large allocations on the main thread. This commit reduces the allocation size and makes a number of other cleanups. The main goal is to hopefully make this test more reliable / not-flaky. Also move the test to sequential. The frequent large allocations could be causing the test to be flaky if run parallel to other tests.
066d836
to
49d5078
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Test has been flaky with timeouts in CI. This is possibly due to the repeated large allocations on the main thread. This commit reduces the allocation size and makes a number of other cleanups. The main goal is to hopefully make this test more reliable / not-flaky. Also move the test to sequential. The frequent large allocations could be causing the test to be flaky if run parallel to other tests. PR-URL: #54839 Refs: #52274 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Landed in 4b2cab2 |
Some tests time out due to a deadlock during process shutdown, see #52550 (comment). I think that the underlying issue is the same here. We could revert/revisit this when a fix lands. |
Test has been flaky with timeouts in CI. This is possibly due to the repeated large allocations on the main thread. This commit reduces the allocation size and makes a number of other cleanups. The main goal is to hopefully make this test more reliable / not-flaky. Also move the test to sequential. The frequent large allocations could be causing the test to be flaky if run parallel to other tests. PR-URL: #54839 Refs: #52274 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Test has been flaky with timeouts in CI. This is possibly due to the repeated large allocations on the main thread. This commit reduces the allocation size and makes a number of other cleanups. The main goal is to hopefully make this test more reliable / not-flaky. Also move the test to sequential. The frequent large allocations could be causing the test to be flaky if run parallel to other tests. PR-URL: #54839 Refs: #52274 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Test has been flaky with timeouts in CI. This is possibly due to the repeated large allocations on the main thread. This commit reduces the allocation size and makes a number of other cleanups. The main goal is to hopefully make this test more reliable / not-flaky. Also move the test to sequential. The frequent large allocations could be causing the test to be flaky if run parallel to other tests. PR-URL: #54839 Refs: #52274 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Test has been flaky with timeouts in CI. This is possibly due to the repeated large allocations on the main thread. This commit reduces the allocation size and makes a number of other cleanups. The main goal is to hopefully make this test more reliable / not-flaky.
Also move the test to sequential. The frequent large allocations could be causing the test to be flaky if run parallel to other tests.
Refs: #52274
update: I realized after that it wasn't clear what was meant by "reduce the allocation size" here. The actual allocation on each iteration remains the same, but by waiting to start allocating until the worker reports the "online" event we reduce (albeit slightly) the number of allocations that are done overall. In local testing this reduced the actually number of allocation calls by a fair amount, reducing the amount of memory the test was using. Unfortunately I don't think just doing that is enough to moving the test to sequential should hopefully also help.