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

Push tag helper checksums into the compiler #9367

Merged
merged 22 commits into from
Oct 11, 2023

Conversation

DustinCampbell
Copy link
Member

Rather than compute tag helper checksums at the tooling layer, this change pushes checksums into the compiler. The biggest benefit of this is that tag helpers now use checksums to test for equality and produce hash codes. So, the old tag helper comparers can be completely deleted, and equality checks should get faster across the board.

@DustinCampbell DustinCampbell requested review from a team as code owners October 5, 2023 19:17
Copy link
Contributor

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

I mainly looked at the tooling changes, but LGTM.

Just for my curiousity, does the checksum builder actually have to be shared? Or is it just to keep the builder and checksum class close to each other. I couldn't see anything in the PR but might have missed it.

@DustinCampbell
Copy link
Member Author

DustinCampbell commented Oct 6, 2023

Just for my curiousity, does the checksum builder actually have to be shared? Or is it just to keep the builder and checksum class close to each other. I couldn't see anything in the PR but might have missed it.

Is there a reason why it shouldn't be in the shared project? It's a general mechanism for building SHA-256 checksums. I don't see a reason they should be separated.

@davidwengier
Copy link
Contributor

Is there a reason why it shouldn't be in the shared project?

No, not at all. I'm just wondering if I missed something in the PR, and there is a scenario where checksums still need to be created on the tooling side.

@DustinCampbell
Copy link
Member Author

No, not at all. I'm just wondering if I missed something in the PR, and there is a scenario where checksums still need to be created on the tooling side.

I just figured that it was a generally useful type, so it was reasonable to go into the shared layer. It's not tied to tag helpers in anyway, and is useful for producing SHA-256 checksums of potentially any object.

@DustinCampbell
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@DustinCampbell DustinCampbell merged commit ea7808b into dotnet:main Oct 11, 2023
12 checks passed
@ghost ghost added this to the Next milestone Oct 11, 2023
@DustinCampbell DustinCampbell deleted the compiler-checksums branch October 11, 2023 18:28
@jjonescz jjonescz modified the milestones: Next, 17.9 P1 Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants