Skip to content

Conversation

@Anipik
Copy link
Contributor

@Anipik Anipik commented Jan 3, 2019

Specifying IterationSetup benchmark causes the benchmark to run just only once in an iteration. This is a problem if the benchmarks takes less than 100ms to execute.

As it is being executed just once per iteration and number of iterations is 20, no jitting is done on netcore3.0 (requires 30 calls).

The possible solutions were to increase the length + increase the warmCount to 30 (this could be a problem for other benchmarks)

Another solution is just removing the iterationSetup. In CacheWithSeeker we are not doing anything that is specific to an iteration and we can directly move the code to globalSetup.

for CacheWithCursor I moved the implementation to the actual benchmark as the iteration setup itseld was taking a small time. But it will good if we could move the iteration setup to globalSetup. @TomFinley any suggetions will be helpful.

Also Earlier this was reported as regression for netcore3.0 but after testing the benchmark with both the solutions, I found there is no regression in this benchmark.

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit:

Copy link
Contributor

@TomFinley TomFinley left a comment

Choose a reason for hiding this comment

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

Very interesting. Thank you for such a complete explanation of the mechanics @Anipik, they were quite enlightening and will I'm sure help me write better benchmarks in future. (The JIT behavior in particular was very interesting reading!)

Regarding your specific question, cache we might prefer in setup to do an initial "pass" to ensure that the data is cached. (It caches on first access, then subsequent accesses will be cached.) But, that would all depend on exactly what the original author intended to test with this benchmark, which is not immediately clear to me. (By which I mean, maybe they were trying to capture the time it took to initially cache? But this we might suppose to be just the time of the original source data view? Who knows.)

@Anipik
Copy link
Contributor Author

Anipik commented Jan 4, 2019

Thanks for the info @TomFinley

@Anipik Anipik merged commit 0d903ab into dotnet:master Jan 4, 2019
@Anipik Anipik deleted the cursor branch January 4, 2019 01:13
@ghost ghost locked as resolved and limited conversation to collaborators Mar 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants