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

RunWithFakeThreshold with DisableParallelization #103423

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

kzrnm
Copy link
Contributor

@kzrnm kzrnm commented Jun 13, 2024

Fix #103247

Non-reproducible assertion errors often occur when tests are run in parallel. I suspect it is due to RunWithFakeThreshold affecting other tests. I hope that applying DisableParallelization to tests that use RunWithFakeThreshold will avoid these errors.

Detail

Debug.Assert(right.Length < MultiplyKaratsubaThreshold);

Multiplication of BigInteger has this assersion.

#if DEBUG
// Mutable for unit testing...
internal static
#else
internal const
#endif
int MultiplyKaratsubaThreshold = 32;

MultiplyKaratsubaThreshold is 32 and the field is immutable in normal execution.

[Fact]
public static void RunMultiply_TwoLargeBigIntegers_Threshold()
{
// Again, with lower threshold
BigIntTools.Utils.RunWithFakeThreshold(BigIntegerCalculator.SquareThreshold, 8, () =>
BigIntTools.Utils.RunWithFakeThreshold(BigIntegerCalculator.MultiplyKaratsubaThreshold, 8, RunMultiply_TwoLargeBigIntegers)
);
// Again, with lower threshold
BigIntTools.Utils.RunWithFakeThreshold(BigIntegerCalculator.SquareThreshold, 8, () =>
BigIntTools.Utils.RunWithFakeThreshold(BigIntegerCalculator.MultiplyKaratsubaThreshold, 8, () =>
BigIntTools.Utils.RunWithFakeThreshold(BigIntegerCalculator.StackAllocThreshold, 8, RunMultiply_TwoLargeBigIntegers)
)
);
}

RunWithFakeThreshold rewrites MultiplyKaratsubaThreshold to 8.

Flow of assertion (my guess)

For the sake of explanation, let {a} denote a Span of length a. For example, {8} is a Span of length 8.

Normal

  1. [testA] BigIntegerCalculator.Multiply({32}, {32})
  2. [testA] BigIntegerCalculator.Multiply.MultiplyKaratsuba({32}, {32})
  3. [testA] BigIntegerCalculator.Multiply.Naive({16}, {16})
  4. [testA] Assert(16 < MultiplyKaratsubaThreshold = 32)

Parallel

  1. [testA] BigIntegerCalculator.Multiply({32}, {32})
  2. [testA] BigIntegerCalculator.Multiply.MultiplyKaratsuba({32}, {32})
  3. [testA] BigIntegerCalculator.Multiply.Naive({16}, {16})
  4. [testB] RunWithFakeThreshold(8,...): MultiplyKaratsubaThreshold = 8 ‼️
  5. [testA] Assert(16 < MultiplyKaratsubaThreshold = 8) ⚠️

@tannergooding tannergooding merged commit 44b4d33 into dotnet:main Aug 12, 2024
81 of 83 checks passed
@kzrnm kzrnm deleted the DisableParallelization branch August 12, 2024 19:51
@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Numerics community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion in BigIntegerCalculator
2 participants