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

Change the DNMD APIs to act on single rows and bring other perf improvements from the CoreCLR + DNMD experiment to DNMD. #55

Merged

Conversation

jkoritzinsky
Copy link
Collaborator

This PR changes the various DNMD column reading/writing APIs to act primarily on 1 row instead of being designed primarily for bulk processing. The vast majority of usages are for single-row operations.

Also bring back a variety of different optimizations that (all in combination and with the changes above) gets an implementation of CoreCLR's IMDInternalImport for read-only usage to be almost 2% faster in my local benchmark.

I'm not bringing that interface into DNMD with this PR as that implementation also requires changes to IMDInternalImport to support DNMD's implementation (and as a result is not compatible with stock CoreCLR). Additionally, IMDInternalImport's implementation is (as the name implies) internal to CoreCLR and not intended for public consumption.

When bringing DNMD into CoreCLR, I'll include the implementation with the requisite changes.

…vements from the CoreCLR + DNMD experiment to DNMD.
Copy link
Owner

@AaronRobinsonMSFT AaronRobinsonMSFT left a comment

Choose a reason for hiding this comment

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

I'm halfway done. There is some sloppiness here that is concerning. I see a great deal of code changes that seem to introduce new code paths that aren't clear why and it is hard to ensure I didn't miss anything.

src/dnmd/access.c Outdated Show resolved Hide resolved
src/dnmd/bytes.c Outdated Show resolved Hide resolved
src/dnmd/search.template.h Outdated Show resolved Hide resolved
src/dnmd/search.template.h Outdated Show resolved Hide resolved
src/dnmd/query.c Outdated Show resolved Hide resolved
src/dnmd/query.c Outdated Show resolved Hide resolved
src/dnmd/query.c Outdated Show resolved Hide resolved
src/dnmd/query.c Outdated Show resolved Hide resolved
src/dnmd/query.c Outdated Show resolved Hide resolved
src/dnmd/query.c Outdated Show resolved Hide resolved
src/interfaces/signatures.cpp Outdated Show resolved Hide resolved
src/interfaces/signatures.hpp Outdated Show resolved Hide resolved
@jkoritzinsky jkoritzinsky merged commit c42d927 into AaronRobinsonMSFT:master Oct 17, 2024
12 checks passed
@jkoritzinsky jkoritzinsky deleted the perftest-results-msvc branch October 17, 2024 21:29
jkoritzinsky added a commit to jkoritzinsky/runtime that referenced this pull request Oct 17, 2024
…vements from the CoreCLR + DNMD experiment to DNMD.

Port of AaronRobinsonMSFT/DNMD#55
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.

2 participants