Skip to content

Update Cursively benchmark code. #1

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 12, 2021
Merged

Update Cursively benchmark code. #1

merged 2 commits into from
Jan 12, 2021

Conversation

airbreather
Copy link
Contributor

@airbreather
Update Cursively benchmark code.
16acc1d 
- VisitPartialFieldContents is required for correctness
- Use the original byte array, since we have one, instead of forcing a stream around it
- Set iteration time to 1 second, to work around dotnet/BenchmarkDotNet#837 while we wait for a version of this package that includes dotnet/BenchmarkDotNet#1573
MarkPflug
MarkPflug reviewed Jan 12, 2021
View reviewed changes
Copy link
Owner

@MarkPflug MarkPflug 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 updates Joe. I've noted one minor tweak you can make to gain a bit of time.

@airbreather
Change the order of this check
e7d0c2b 
Whether or not this is the optimal order is almost certain to depend on the data: data sets that would almost never use the pool would likely hurt more by doing it this way, especially if this were a real-world application since this would likely push a line out of the CPU cache for no reason.

Then again, a real-world application probably wouldn't use the pool conditionally like this, so there's not much of a reason NOT to either.
@MarkPflug
Copy link
Owner

Thanks again. I'll get the benchmarks table updated with the results soon.

@MarkPflug MarkPflug merged commit ba6a1b1 into MarkPflug:main Jan 12, 2021
@MarkPflug
Copy link
Owner

I just pushed an update to the reports, reorganizing things a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarkPflug MarkPflug MarkPflug left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@airbreather @MarkPflug