Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add benchmarks #50

Closed
wants to merge 1 commit into from
Closed

Conversation

Konippi
Copy link
Contributor

@Konippi Konippi commented Jul 28, 2024

Add comprehensive benchmarks for FxHashMap

This PR adds a comprehensive benchmarks for the FxHashMap implementation.
These benchmarks cover various operations and scenarios to provide a thorough performance evaluation.

Key changes:

  1. Added benchmarks for basic operations:

    • Creating an empty FxHashMap
    • Inserting items
    • Looking up items
    • Removing items
    • Iterating over the map
    • Reinserting items (updating existing keys)
  2. Included benchmarks for operations with large datasets:

    • All basic operations are also benchmarked with a large number of items
  3. Added benchmarks for concurrent scenarios:

    • Using FxHashMap with a Mutex
    • Using FxHashMap with a RwLock
  4. Added conditional compilation to allow benchmarking against the standard library's HashMap for comparison:

    • Use the std_bench feature flag to run benchmarks for std::collections::HashMap

Benefits:

These benchmarks will help us:

  • Monitor the performance of FxHashMap over time
  • Compare FxHashMap's performance against other hash map implementations
  • Identify potential areas for optimization

fix: fmt & exclude benchmarks from ci

fix: fmt
@workingjubilee
Copy link
Member

These benchmarks have several problems:

  • they always iterate over the hashmap in the exact same order
  • they always use integer keys in ascending order
  • bench_hashmap_remove_large_data calls String::drop in a hot loop

There's more, but please explain why you are benchmarking each thing. Yes, so that it goes fast, but what parts does it test? Is there something it also winds up testing that it shouldn't? What parts are just testing the hashmap implementation instead of the hash?

@Konippi
Copy link
Contributor Author

Konippi commented Jul 30, 2024

Thanks for your accurate review!
If I focus my benchmark testing only on hash functions, do you think that is necessary?
If not, I will consider closing this PR...

@Noratrieb
Copy link
Member

I think it makes sense to close this PR. The primary purpose of this crate is to serve the Rust compiler, which already has an extensive benchmark suite, so any changes to this will need to be benchmarked against that anyways, so I don't think these benchmarks here provide that much value, and could be deceptive.
Thank you for spending time trying to improve this anyways!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants