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

Use 64-bit hashes regardless of platform #1918

Merged
merged 1 commit into from
Dec 5, 2023
Merged

Use 64-bit hashes regardless of platform #1918

merged 1 commit into from
Dec 5, 2023

Conversation

TimSylvester
Copy link
Collaborator

@TimSylvester TimSylvester commented Dec 5, 2023

Per #1914, use uint64_t for hash keys instead of size_t for more consistent hash results across platforms.

The unit tests check the shader definitions for collisions alongside testing the hash algorithm with random values, but (so far) only in Metal builds where that doesn't require reflection.

Resolves #1917

Copy link

github-actions bot commented Dec 5, 2023

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0% +3.41Ki  +0.0%    +328    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-1918-compared-to-main.txt

Compared to d387090 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +18% +21.4Mi  +399% +23.8Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-1918-compared-to-legacy.txt

Copy link

codecov bot commented Dec 5, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a92bc67) 85.74% compared to head (b6b1c55) 85.75%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1918      +/-   ##
==========================================
+ Coverage   85.74%   85.75%   +0.01%     
==========================================
  Files         568      568              
  Lines       28026    28027       +1     
==========================================
+ Hits        24031    24035       +4     
+ Misses       3995     3992       -3     

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

@TimSylvester TimSylvester merged commit f7666f4 into main Dec 5, 2023
31 checks passed
@TimSylvester TimSylvester deleted the 64bit-hash branch December 5, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Use 64-bit hashes on 32-bit platforms.
4 participants