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

CQ fix for generic handle lookup in GenericHandleWorker #618

Merged
merged 1 commit into from Apr 2, 2015

Conversation

Projects
None yet
4 participants
@cmckinsey
Contributor

cmckinsey commented Apr 2, 2015

When looking up the runtime handle in the generic handle cache for a methodtable
we get the declaring methodtable in the hierarchy and then lookup the handle in
the cache from that methodtable. When found we should insert the handle back into
the table using the original methodtable as the key and not the declaring MT
so that later lookups are faster. Inserting the handle back into the table under
the original key was a noop.

Desktop DDR and JITSH testing clean. The test case in Github issue #55 is now
3x faster. Benchmarked roslyn performance which shows no change in performance.

This is a fix related to issue #55 reported by alexandr. See for more details.
@jkotas @briansull

CQ fix for generic handle lookup in GenericHandleWorker
When looking up the runtime handle in the generic handle cache for a methodtable
we get the declaring methodtable in the hierarchy and then lookup the handle in
the cache from that methodtable. When found we should insert the handle back into
the table using the original methodtable as the key and not the declaring MT
so that later lookups are faster. Inserting the handle back into the table under
the original key was a noop.

Desktop DDR and JITSH testing clean. The test case in Github issue #55 is now
3x faster. Benchmarked roslyn performance which shows no change in performance.
@jkotas

This comment has been minimized.

jkotas commented on df886a5 Apr 2, 2015

LGTM. Good catch 👍

@briansull

This comment has been minimized.

Contributor

briansull commented Apr 2, 2015

LGTM to me as well.

jkotas added a commit that referenced this pull request Apr 2, 2015

Merge pull request #618 from cmckinsey/coreclr55
CQ fix for generic handle lookup in GenericHandleWorker

@jkotas jkotas merged commit 9404836 into dotnet:master Apr 2, 2015

1 check passed

default Merged build finished.
Details

@cmckinsey cmckinsey deleted the cmckinsey:coreclr55 branch Apr 3, 2015

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