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

RuntimeBinder Memory leaks #308

Closed
DmitryDem opened this issue Aug 1, 2019 · 14 comments · Fixed by #359
Closed

RuntimeBinder Memory leaks #308

DmitryDem opened this issue Aug 1, 2019 · 14 comments · Fixed by #359

Comments

@DmitryDem
Copy link

Method GetProperty(object target, string name) in the PathBinder.cs and IteratorBinder.cs cause memory leak in highload application. Could you use another way for getting property value instead of using RuntimeBuilder class?
image

@rexm
Copy link
Member

rexm commented Aug 1, 2019

PR welcome

@DmitryDem
Copy link
Author

Hi!
Maybe we can use ExpressionTrees instead RuntimeBuilder?

For example:
var parameter = Expression.Parameter(typeof(object));
var cast = Expression.Convert(parameter, value.GetType());
var propertyGetter = Expression.Property(cast, propertyName);
var castResult = Expression.Convert(propertyGetter, typeof(object));//for boxing
var propertyRetriver = Expression.Lambda<Func<object, object>>(castResult, parameter).Compile();
var retrivedPropertyValue = propertyRetriver(value);

I know that's not a cheap operation because expression threes should be compiled. But i think that's it should be faster then DLR. What do you think?

@rexm
Copy link
Member

rexm commented Aug 1, 2019 via email

@DmitryDem
Copy link
Author

Yea, sorry, didn't notice

@DmitryDem
Copy link
Author

DmitryDem commented Aug 1, 2019

What do you think about CallSite caching? I think it's should help. The main problem occurs when creating CallSite objects.
For example:

internal static class CallSiteCache
    {
        private static readonly ConcurrentDictionary<string, CallSite<Func<CallSite, object, object>>> getters = new ConcurrentDictionary<string, CallSite<Func<CallSite, object, object>>>();

        internal static object GetValue(object target, string name)
        {
            var callSite = getters.GetOrAdd(name, CreateSite(target, name));
            return callSite.Target(callSite, target);
        }

        internal static CallSite<Func<CallSite, object, object>> CreateSite(object target, string name)
        {
            return CallSite<Func<CallSite, object, object>>.Create(Binder.GetMember(CSharpBinderFlags.None, name, target.GetType(), new[] { CSharpArgumentInfo.Create(CSharpArgumentInfoFlags.None, null) }));
        }
    }

@DmitryDem
Copy link
Author

I don't understand how this api can help. Usually this api uses for custom DynamicObject implementation and contains describing get methods of custom DynamicObject. Could you please clarify idea for using this api.

@DmitryDem
Copy link
Author

DmitryDem commented Aug 2, 2019

I have just implemented some solution. What do you think?

 private static object GetProperty(IDynamicMetaObjectProvider target, string name)
        {
            var self = (Expression)Expression.Constant(target);
            var metaObject = target.GetMetaObject(self);
            var binder = (GetMemberBinder)Binder.GetMember(CSharpBinderFlags.None, name, target.GetType(), new[] { CSharpArgumentInfo.Create(CSharpArgumentInfoFlags.None, null) });
            var member = metaObject.BindGetMember(binder);
            var final = Expression.Block(
                Expression.Label(CallSiteBinder.UpdateLabel),
                member.Expression
            );
            var param = Expression.Parameter(typeof(object));
            var lambda = Expression.Lambda(final, param);
            var @delegate = lambda.Compile();
            return @delegate.DynamicInvoke(target);
        }

@rexm
Copy link
Member

rexm commented Aug 2, 2019

That all makes sense to me except I don’t entirely understand the Lambda wrapper bit - can you explain?

@DmitryDem
Copy link
Author

Yes lambda wrapper could be simplified. I think we could replace Lambda wrapper with this

var lambda = Expression.Lambda<Func<object>>(final).Compile();
            return lambda();

These lines do not make sense.

 var param = Expression.Parameter(typeof(object));
            var lambda = Expression.Lambda(final, param);

@rexm
Copy link
Member

rexm commented Aug 2, 2019

The DynamicMetaObject has other properties that look like they give direct access to the underlying value without creating a new expression tree - take a look at the Properties section on https://docs.microsoft.com/en-us/dotnet/api/system.dynamic.dynamicmetaobject?view=netframework-4.8

@DmitryDem
Copy link
Author

Do you mean "Value" property? It does't work for ExpandoObject. It just return null.
image
Instead of we can call Expression that contains logic access to the member
image
Every custom DynamicMetaObject that inherited from DynamicMetaObject can override BindGetMember and provide Expression for access to their members.
Look at example for JObject from Json.NET library
image
If you know another way I will be grateful for the help

@rexm
Copy link
Member

rexm commented Aug 2, 2019

ExpandoObject implements IDictionary<string,object> so you may want to consider reordering the type checks to give that precedence. The structure of the routine should be that if a value isn't resolvable after one attempt, fall back up and keep trying others.

@XcoderBlood
Copy link

Was this ever fixed?

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 a pull request may close this issue.

3 participants