Skip to content

Add 64-bit non crypto hash algo for dgspec uniqueness computation#5655

Merged
jeffkl merged 2 commits intoNuGet:devfrom
GenelleM:dev-genellem-msft-UsenoncryptographicHash
Mar 5, 2024
Merged

Add 64-bit non crypto hash algo for dgspec uniqueness computation#5655
jeffkl merged 2 commits intoNuGet:devfrom
GenelleM:dev-genellem-msft-UsenoncryptographicHash

Conversation

@GenelleM
Copy link
Copy Markdown
Contributor

@GenelleM GenelleM commented Feb 29, 2024

Bug

Fixes: NuGet/Home#13214

Regression? Last working version:

Description

Switch calculating dependency graph spec json to a 64-bit non-crypto hash from SHA512 hash to reduce comparison/restore time but maintaining accuracy of need to restore when determining if NuGet needs to regenerate the dgspec.json file.

We've also added an escape hatch a user can set the environment variable NUGET_ENABLE_LEGACY_DGSPEC_HASH_FUNCTION to true to bring back the SHA512 hash function.

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception
    • OR
    • N/A
  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

@GenelleM GenelleM requested a review from a team as a code owner February 29, 2024 00:49
@dotnet-policy-service dotnet-policy-service bot added the Community PRs created by someone not in the NuGet team label Feb 29, 2024
@GenelleM
Copy link
Copy Markdown
Contributor Author

@jeffkl

Copy link
Copy Markdown
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

How do the numbers look like before/after the change?
How much of a difference is this making?

@jeffkl jeffkl force-pushed the dev-genellem-msft-UsenoncryptographicHash branch from ee6861a to 05a55b0 Compare March 1, 2024 21:17
@jeffkl jeffkl self-assigned this Mar 1, 2024
@jeffkl jeffkl requested a review from nkolev92 March 1, 2024 21:22
nkolev92
nkolev92 previously approved these changes Mar 4, 2024
Copy link
Copy Markdown
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

Let's add the env var just in case and 🚢 it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community PRs created by someone not in the NuGet team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProjectModel.HashObjectWriter.OnFlush is using a SHA512 hash versus a cheaper hash which seems like overkill

3 participants