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

Fixed dlhn benchmark to work properly #6

Merged
merged 1 commit into from
May 6, 2023

Conversation

otake84
Copy link
Contributor

@otake84 otake84 commented May 6, 2023

Thank you for choosing dlhn as a benchmark target. I have made the following changes to make the dlhn benchmark work:

  1. Changed the type of the LargeStruct map field from HashMap<u32, u64>, to HashMap<String, u64>. This is because dlhn can only use strings as Map types.
  2. In Rust, floating-point numbers cannot be used as keys for HashMap or BTreeMap, so we made a bold decision to design dlhn to only allow strings as keys for Maps.
  3. Currently, dlhn does not support 128-bit integers, so I have made changes to disable the dlhn benchmark when the model_128 option is enabled.
  4. Lastly, I fixed the code to work by uncommenting benches!(serde_cbor);, which was commented out along with the fixme comment.

I hope that these changes will help improve the benchmarking process for musli. Thank you for your support and understanding.

@udoprog
Copy link
Owner

udoprog commented May 6, 2023

Seems reasonable to me. Thanks for the rundown!

@udoprog udoprog merged commit 3e9cb7e into udoprog:main May 6, 2023
@udoprog udoprog added the enhancement New feature or request label May 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants