Skip to content

Conversation

@airbreather
Copy link
Contributor

- 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
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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants