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

Add JsonNode construction benchmarks #4463

Merged
merged 2 commits into from
Sep 18, 2024

Conversation

eiriktsarpalis
Copy link
Member

Contributes to dotnet/runtime#107869

LoopedBard3
LoopedBard3 previously approved these changes Sep 17, 2024
Copy link
Member

@LoopedBard3 LoopedBard3 left a comment

Choose a reason for hiding this comment

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

I think this is a good start, although we will probably want to put the super fast benchmarks (<100ns per iteration) into a loop or something to get their mean time closer to 200-300ns in order to help minimize noisiness. If you have an idea of how to do this while keeping the tests inline with their goal and can make the change, that would be great, otherwise doing this exercise is one of the items we want to do.

@LoopedBard3
Copy link
Member

@eiriktsarpalis Left a comment and approved the PR, whether you include the change mentioned in the approval, ping me when you are ready for this to be merged and I can merge it.

@eiriktsarpalis
Copy link
Member Author

Something like 26e65c2?

@LoopedBard3
Copy link
Member

Yes, exactly like that 👍. Thanks! Just to double check, are you good having this merged once the tests run?

@eiriktsarpalis
Copy link
Member Author

Yep, go for it!

@LoopedBard3
Copy link
Member

ubuntu Micro failure is related to a cryptographic error, not to any changes in this PR. Merging.

@LoopedBard3 LoopedBard3 merged commit f6d0cff into dotnet:main Sep 18, 2024
51 of 57 checks passed
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