Faster writing of Json-objects by not boxing properties

[snakefoot]

Right now the object-reflection is rather simple, and converts all properties into object. Then afterwards the object is serialized into the expected format (json / xml / etc.)

When handling properties of value-type, then this means boxing (memory allocation). Could be nice if the serializer could perform the object-reflection without boxing all value-type-properties.

[bruce-dunwiddie]

@snakefoot , just to confirm before I take a look at this, you're referring to NLog.Targets.DefaultJsonSerializer.SerializeObjectProperties(),

NLog/src/NLog/Targets/DefaultJsonSerializer.cs

Line 708 in 2c6b59e

private bool SerializeObjectProperties(ObjectReflectionCache.ObjectPropertyList objectPropertyList, StringBuilder destination, JsonSerializeOptions options,

?

[snakefoot]

Yes that is the spot.

[bruce-dunwiddie]

And are you ok with an implementation that basically is just a switch statement with specific handling for each primitive value type? I think that's basically how I've seen this handled in various other libraries.

And do you have a current size or complexity that you've seen this have speed issues, or is it just something you know could or should be faster? I'll be setting up some kind of proof of speed improvement to verify changes, but would like to know any context you have.

[snakefoot]

Think it would be a great start if there was handling of ValueTypes that implements IConvertible without boxing (int, bool, double, decimal, datetime etc.)

Guess one can do the performance test with:

    public class SomeData
    {
        public int MyProperty1 { get; set; } = 1;
        public double MyProperty2 { get; set; } = 1.4;
        public DateTime MyProperty3 { get; set; } = DateTime.Now;
        public bool MyProperty4 { get; set; } = true;
        public string MyProperty5 { get; set; } = "my value";
        public SomeData MyProperty6 { get; set; }
    }

[bruce-dunwiddie]

Because the process to get the value of the property within the cache lookup logic has to use PropertyInfo.GetValue(Object, Object[]), that returns System.Object and does not offer a generic PropertyInfo.GetValue<T>() version, then the primitive value will already be boxed, and I am unaware of a way to get around that.

If you were to find a way, you would then also have to modify the PropertyValue struct that is being returned from the cache lookup to create some form of generic <T> implementation, and you won't be able to cast the value into an IConvertible, because that will also cause boxing.

[bruce-dunwiddie]

I suppose you might be able to compile an Expression, specific to the property, then provide a GetValueExpression<T>() method on the PropertyValue struct, that could then be called from within a generic method inside the serializer. I don't think this would box the value. It would be quite a rework of code though. Is this a path you'd want to go down?

[snakefoot]

Yes somehow the ObjectReflectionCache needs to build an object-serialization-contract (that is cached), so the object-reflection-cache can query the json-serializer what types supports "non-boxed"-serialization (only once for each type)

So the json-serializer must implement a method that receives a PropertyInfo and can then resolve to a expression-tree-lambda (static method that takes object-value and json-serializer and stringbuilder as input parameter).

When json-serializer enumerates the special-property-list (object-serialization-contract) then it will call a method like propertyValue.TrySerialize(this) and if the special-property-list has resolved a serializer-method, then it is used and returns true. But when TrySerialize returns false, then standard serialization happens as normal (of boxed value)

Anyway just an idea, there are probably better and more elegant ways (Not an expert in building serializers or using expression-trees)

[bruce-dunwiddie]

The code needing to be changed keeps going deeper and deeper.

There are 3 constructors in PropertyValue, and 2 of them have already boxed the primitive values prior to the call to the constructor.

                public PropertyValue(string name, object value, TypeCode typeCode)
                {
                    Name = name;
                    Value = value;
                    _typecode = typeCode;
                }

                public PropertyValue(object owner, PropertyInfo propertyInfo)
                {
                    Name = propertyInfo.Name;
                    Value = propertyInfo.GetValue(owner, null);
                    _typecode = TypeCode.Object;
                }

                public PropertyValue(object owner, FastPropertyLookup fastProperty)
                {
                    Name = fastProperty.Name;
                    Value = fastProperty.ValueLookup(owner, null);
                    _typecode = fastProperty.TypeCode;
                }

The first constructor is used in ObjectPropertyList.Enumerator, which has a few layers of containers that have all boxed to object already.

The second constructor could be left alone from the caller's perspective, but it appears to only be used for non primitive values because it's hardcoded to set the TypeCode to Object.

The FastPropertyLookup's current expression tree used in the third constructor also boxes values.

I feel that essentially the entirety of ObjectReflectionCache, and all the associated classes, will have to be redone from scratch to ensure a path of preventing the boxing, and then build back in all the various handling for different object and property types. I'm not sure how many of the special cases are covered in the current JSON serializer tests?

[snakefoot]

Essentially the entirety of ObjectReflectionCache, and all the associated classes, will have to be redone from scratch to ensure a path of preventing the boxing

Also my observation. The ObjectReflectionCache was a very naive attempt to support structured-logging. Could also be that we should focus on opening NLog to make use of System.Text.Json, since it has the ability to serialize without boxing. But it seems that System.Text.Json is very fragile, and explodes when not basic datatypes.