dataobjects-net icon indicating copy to clipboard operation
dataobjects-net copied to clipboard

Memoize InnerGetTypeName()

Open SergeiPavlov opened this issue 2 years ago • 6 comments

We see in tracing this is a hotspot

SergeiPavlov avatar Mar 23 '23 20:03 SergeiPavlov

Is ths a hotspot during Domain building or during domain live?

alex-kulakov avatar Aug 11 '23 14:08 alex-kulakov

I don't remember. By be just to make the code optimal for any case.

SergeiPavlov avatar Aug 11 '23 16:08 SergeiPavlov

I think I know when you get hot paths for GetShortName(). Usage during Domain build corelates with number of model nodes (TypeInfo, FieldInfo, IndexInfo and TypeDef, FieldDef and so on). On HugeModeUpgrade.RegularModel I have 615 calls of GetShortName during domain build. Real problem is queries, especially TranslatorContext.GetApplyParameter(). During validation of populated data, tests execute queries and the validation queries make 2600 additional calls of GetShortName.

In various cases we can replace GetShortName with System.Type.Name, it is valid replacement for types which are guaranteed to be not Nested and not GenericType. Providers are among them. I have found more cases when we can make such replacement.

I see that System.RuntimeType has cache for names (according to the sources)

        public override String Name 
        {
            get 
            {
                return GetCachedName(TypeNameKind.Name);
            }
        }

Implicit usage of this cache will help us to not bloat our caches.

Maybe we can start with something like this (#333) and see how it goes. What do you think? I don't know whether you can check results of the PR I mentioned and tell me the outcome.

alex-kulakov avatar Aug 15 '23 13:08 alex-kulakov

I don't mind these changes, but to test them with our app requires big efforts to create special branches and build. So I'm going to wait until they will be merged into main branch

SergeiPavlov avatar Aug 15 '23 19:08 SergeiPavlov

Sure, as I thought, it is understandable.

alex-kulakov avatar Aug 16 '23 05:08 alex-kulakov

I merged my PR, I postpone this one for results of my PR, whether it did something or not.

alex-kulakov avatar Aug 16 '23 13:08 alex-kulakov