Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Unify caching pattern between EcmaMethod and EcmaField#19

Merged
jkotas merged 2 commits into
dotnet:masterfrom
jkotas:lazy-init
Oct 2, 2015
Merged

Unify caching pattern between EcmaMethod and EcmaField#19
jkotas merged 2 commits into
dotnet:masterfrom
jkotas:lazy-init

Conversation

@jkotas
Copy link
Copy Markdown
Member

@jkotas jkotas commented Oct 2, 2015

I have looked a bit at the right pattern to use for caching of metadata {Field|Method}DefinitionHandle vs. {Field|Method}Definition: The conversion from XXXDefinitionHandle to XXXDefinition is cheap. It is sufficient to cache just XXXDefinitionHandle, and recompute XXXDefinition each time - updated EcmaMethod implementation to match.

Also, I have removed MethodDefinition and FieldDefiniton helper properties from EcmaMethod and EcmaField because of they were confusing (and JIT have not generated a particularly good code for them).

@MichalStrehovsky
Copy link
Copy Markdown
Member

LGTM

jkotas added 2 commits October 2, 2015 11:21
I have looked a bit at the right pattern to use for caching of metadata {Field|Method}DefinitionHandle vs. {Field|Method}Definition: The conversion from XXXDefinitionHandle to XXXDefinition is cheap. It is sufficient to cache just XXXDefinitionHandle, and recompute XXXDefinition each time - updated EcmaMethod implementation to match.

Also, I have removed MethodDefinition and FieldDefiniton helper properties from EcmaMethod and EcmaField because of they were confusing (and JIT have not generated a particularly good code for them).
jkotas added a commit that referenced this pull request Oct 2, 2015
Unify caching pattern between EcmaMethod and EcmaField
@jkotas jkotas merged commit 648dff6 into dotnet:master Oct 2, 2015
@jkotas jkotas deleted the lazy-init branch October 12, 2015 06:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants