Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Fix DeflateStream Performance test #21050

Merged
merged 1 commit into from
Jun 14, 2017
Merged

Fix DeflateStream Performance test #21050

merged 1 commit into from
Jun 14, 2017

Conversation

Vedin
Copy link
Contributor

@Vedin Vedin commented Jun 14, 2017

No description provided.

@stephentoub
Copy link
Member

@Vedin, can you briefly explain what the bug was and how this fixes it?

cc: @ianhays

@Vedin
Copy link
Contributor Author

Vedin commented Jun 14, 2017

@stephentoub, Right now, we enter the while (retCount != 0) loop once. After you break (after first iteration of benchmark), retCount will be 0 and you will not enter the loop again for subsequent Benchmark.Iterations.
I have just put it inside the loop

[Benchmark]
private const int Iter = 1000;

[Benchmark(InnerIterationCount = Iter)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is reducing the number of inner iterations from 10000 to 1000. Is that on purpose, and if so, why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below

@Vedin
Copy link
Contributor Author

Vedin commented Jun 14, 2017

@ianhays Because on 10000 inner iterations we'll have only 1 Iteration of Benchmark, which doesn't show as an ok average time. So I reduced it to have more than 1 iteration of Benchmark and get a stable results.
image

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. Makes sense to me. Ian?

Copy link
Contributor

@ianhays ianhays left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, thanks for elaborating @Vedin. LGTM

@ianhays ianhays merged commit 051883e into dotnet:master Jun 14, 2017
@karelz karelz modified the milestone: 2.1.0 Jun 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants