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 HashCode not being Case Insensitive, Improved HashCode Performance & Use SIMD/Intrinsics Code from External Library (Fixes #25) #31

Merged
merged 4 commits into from
Nov 23, 2023

Conversation

Sewer56
Copy link
Member

@Sewer56 Sewer56 commented Nov 20, 2023

Changelog

Replaced String Hash Function with Custom One tailored for Lowercase

  • Optimised for ASCII inputs hashing as lower in-place. non-ASCII inputs fallback to first converting to lowercase before hashing.

  • To give some context, for strings, .NET uses a cryptographically secure, DoS resistant hash function.

    • However it's piss slow...
  • It slow enough that Dictionary and HashSet are hardcoded in the runtime to use a different, non-DoS resistant function if the key is typeof(string). (Hint: I checked that function out, it's DJB2 w/ 2 hashes, mixed at the end)

  • Unfortunately you can't use that function 😊 at all, because it's internal, and because RelativePath / AbsolutePath aren't string, you're forced to use the slow one; which is a no-no for us.

  • For a typical AbsolutePath to a file within game folder (~120 characters), the improvement in hashing speed is ~8-12x. (Depending on AVX2 availability)

  • For a typical RelativePath for a file within game folder, or a file segment in a dictionary (~8-12 characters), the improvement is around ~2.3x.

  • Hash quality is a bit lower/more collision prone, however the time save on hashing heavily outweighs the penalty spent on the occasional moment there's 1 more linear probe in .NET Dictionary (BCL dict uses linear probing). This applies even when used with short <8 character file paths.

  • For the curious, you can find the relevant benchmarks here.

Moved fancy SIMD/Intrinsics Code

  • Removed fancy SIMD and Intrinsics code from this library.

  • Now using the originals from Reloaded.Memory, where I originally copied them from.

  • For context: It's a lightweight, high performance library for low level memory manipulation & zero cost abstractions, created/maintained by me. Mostly used in game mods & tools [around 200 projects use it], and also has stable API.

  • This should make maintenance easier, hopefully. (Removes ~1700 LoC from the repo)

  • The only new code that was specifically written for this PR is the aforementioned lowercase variant of custom string hash.

  • Note: This is a breaking change, because some of these were public APIs previously. However fixing any relevant code should be easy as the API names should not have changed, so it's just a matter of adding/replacing a namespace.

Improved AbsolutePath.Equals

  • Changed order of Equals comparisons in AbsolutePath.Equals.
    • Now compares FileName first, instead of directory.
    • Statistically speaking, the file name is more likely to be different faster than directory is. So this is an improvement.

Other

  • Added Sanity Tests to ensure GetHashCode and Equals are case-insensitive for RelativePath & AbsolutePath.

@Sewer56 Sewer56 requested a review from a team November 20, 2023 14:02
@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (29ddf3f) 85.64% compared to head (e5aaa7a) 86.71%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #31      +/-   ##
==========================================
+ Coverage   85.64%   86.71%   +1.07%     
==========================================
  Files          44       42       -2     
  Lines        2939     2861      -78     
  Branches      507      494      -13     
==========================================
- Hits         2517     2481      -36     
+ Misses        357      324      -33     
+ Partials       65       56       -9     
Flag Coverage Δ
Linux 86.29% <100.00%> (+1.06%) ⬆️
Windows 86.29% <100.00%> (+1.06%) ⬆️
macOS 86.22% <100.00%> (+1.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Al12rs Al12rs requested a review from halgari November 20, 2023 15:38
@halgari halgari merged commit 9aff411 into main Nov 23, 2023
5 checks passed
@halgari halgari deleted the fix-hashcode-equals branch November 23, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants