4
\$\begingroup\$

I have a 'speed problem' with a program im currently developing. I already have encountered the methods which costs some time.

public void ConvertDataFrameToValueFields()
 {
 List<byte[]> items = new List<byte[]>() { new byte[2], new byte[2], new byte[2], new byte[2] };
 items[0][0] = this.DataBytes[0];
 items[0][1] = this.DataBytes[1];
 items[1][0] = this.DataBytes[2];
 items[1][1] = this.DataBytes[3];
 items[2][0] = this.DataBytes[4];
 items[2][1] = this.DataBytes[5];
 items[3][0] = this.DataBytes[6];
 items[3][1] = this.DataBytes[7];
 int i = 0;
 IEnumerable<PropertyInfo> result = this.determineCanMessageValueFields();
 foreach (var prop in result)
 {
 if (prop.PropertyType == typeof(MessageStatusValueField))
 {
 prop.SetValue(this, new MessageStatusValueField(BitConverter.ToUInt16(items[i], 0), prop.Name, this.UnitType), null);
 }
 else if (prop.PropertyType == typeof(MessageValueField))
 {
 prop.SetValue(this, new MessageValueField(BitConverter.ToUInt16(items[i], 0), prop.Name), null);
 }
 else if (prop.PropertyType == typeof(MessageValueFieldInt32))
 {
 prop.SetValue(this, new MessageValueFieldInt32(BitConverter.ToUInt16(items[i], 0), prop.Name), null);
 }
 else if (prop.PropertyType.GetCustomAttributes(false).Count(x => x.GetType() == typeof(BitmaskValueFieldClass)) > 0)
 {
 var tempField = new MessageValueField(BitConverter.ToUInt16(items[i], 0), prop.Name);
 prop.SetValue(this, Convert.ChangeType(tempField, prop.PropertyType), null);
 }
 i++;
 }
 }
 private IEnumerable<PropertyInfo> determineCanMessageValueFields()
 {
 //Get all properties with the 
 //custom attribute [IsValueField]
 return this.GetType().GetProperties()
 .Where(p => p.GetCustomAttributes(false)
 .Count(x => x.GetType() == typeof(IsValueField)) > 0)
 .OrderBy(x => ((IsValueField)x.GetCustomAttributes(false)[0]).ValueFieldIndex);
 }
 public object ToType(Type conversionType, IFormatProvider provider)
 {
 //...only convert if the conversion type is a BitmaskValueFieldClass
 if (conversionType.GetCustomAttributes(false).Count(x => x.GetType() == typeof(BitmaskValueFieldClass)) == 0)
 {
 throw new NotImplementedException("The conversion is only available for conversion types with the BitmaskValueFieldClass attribute");
 }
 //...create an instance of the conversion type
 var instance = Activator.CreateInstance(conversionType);
 //...go thru each property and set the value
 foreach (var item in instance.GetType().BaseType.GetProperties())
 {
 item.SetValue(instance, this.GetType().GetProperty(item.Name).GetValue(this, null), null);
 }
 return instance;
 }

The method ConvertDataFrameToValueFields() is the sticking point. I know it is because of the prop.SetValue() and the ToType() IConvertible method calls. When i am removing the ConvertDataFrameToValueFields() method i gain the missing time i need to fulfill the project.

Is there a way to improve the whole thing? Problem is that the idea has to be as it is. In the ConvertDataFrameToValueFields() method i have to map the returning DataFrame, the DataBytes array, to the MessageValueField properties of the specific class. Im getting all containing messagevaluefields by calling the ConvertDataFrameToValueFields().

Edit #1

Some further Info: The ConvertDataFrameToValueFields Method is called several times in the program life cycle. I have an object which contains at first the DataBytes array. Now i have to map the values of the DataBytes array to the specific four properties. this in the ConvertDataFrameToValueFields method points not always the same type of class! It can be type of ClassA or ClassB or ClassC. ClassA, ClassB and ClassC have different properties with the custom attribute IsValueField. Thats why i call determineCanMessageValueFields. I need to get the four specific properties, get the type of the property, and set the value based on the property type. I do this several times in the program life cycle and this is not always the same type. I hope this makes it a little bit clearer.

In Short:

DataBytes array has been filled --> Get the message value fields of the current object the DataBytes have been filled --> Determine the type of the specific property --> Set the value with the specific DataByte value(s)

asked May 7, 2014 at 12:50
\$\endgroup\$
5
  • 2
    \$\begingroup\$ Check this out \$\endgroup\$ Commented May 7, 2014 at 13:08
  • 1
    \$\begingroup\$ The NotImplementedException is misleading. ToType should throw a NotSupportedException for a typeof(BitmaskValueFieldClass). \$\endgroup\$ Commented May 9, 2014 at 14:55
  • \$\begingroup\$ Thanks for the input, but changing the type of exception does not lead to a performance upgrade. \$\endgroup\$ Commented May 12, 2014 at 7:05
  • \$\begingroup\$ What happens, when result.size() > 4? Your code probably would break with an IndexOutOfRangeException. \$\endgroup\$ Commented May 12, 2014 at 7:48
  • \$\begingroup\$ Could you put the whole class for review? It would be great to see the properties you have in order to understand better. Because right now I have a hard time understanding why your class would use reflection on itself. \$\endgroup\$ Commented Oct 7, 2014 at 13:19

1 Answer 1

2
\$\begingroup\$

first of all reflection is expensive when used often. What you can do is

  • cache the properties to set

    Dictionary<Type, IEnumerable<PropertyInfo>> _propertiesCache
    
  • remove the copying over by creating the result immediatly and let it parse it itself

    public TMessage ConvertDataFrameToValueFields<TMessage>(byte[] dataBytes, Whatever unitType) where TMessage : new()
    {
     var result = new TMessage();
     ...
     // use the constructor which takes a ushort and the propertyName. The class itself has to know how to interpret the ushort
     fieldValue = Activator.CreateInstance(prop.PropertyType, value, prop.Name);
     ...
    }
    
  • limit the calls to GetCustomAttributes by using the result twice

    var isValueField = prop.GetCustomAttributes(false).OfType<IsValueFieldAttribute>().FirstOrDefault();
    if (isValueField != null)
     results.Add(isValueField.FieldIndex, prop);
    
  • remove the conversion of byte[] to List because it is not needed. Convert takes the startindex anyway

  • remove the need to store the bytes in the class

and together it will be

class ByteConverter
{
 private readonly Dictionary<Type, IEnumerable<PropertyInfo>> _propertiesCache = new Dictionary<Type, IEnumerable<PropertyInfo>>();
 public TMessage ConvertDataFrameToValueFields<TMessage>(byte[] dataBytes, Whatever unitType) where TMessage : new()
 {
 var result = new TMessage();
 int i = 0;
 foreach (var prop in GetMessageValueFields<TMessage>())
 {
 var value = BitConverter.ToUInt16(dataBytes, i);
 object fieldValue = null;
 if (prop.PropertyType == typeof(MessageStatusValueField))
 {
 fieldValue = new MessageStatusValueField(value, prop.Name, this.UnitType);
 }
 else if (prop.PropertyType == typeof(MessageValueField))
 {
 fieldValue = new MessageValueField(value, prop.Name);
 }
 else if (prop.PropertyType == typeof(MessageValueFieldInt32))
 {
 fieldValue = new MessageValueFieldInt32(value, prop.Name);
 }
 else if (prop.PropertyType.IsSubclassOf(typeof(BitmaskValueFieldClass)))
 {
 // use the constructor which takes a ushort and the propertyName. The class itself has to know how to interpret the ushort
 fieldValue = Activator.CreateInstance(prop.PropertyType, value, prop.Name);
 }
 if (fieldValue != null)
 prop.SetValue(result, fieldValue, null);
 i += 2;
 }
 return result;
 }
 private IEnumerable<PropertyInfo> GetMessageValueFields<TMessage>()
 {
 var type = typeof(TMessage);
 IEnumerable<PropertyInfo> properties;
 if (!_propertiesCache.TryGetValue(type, out properties))
 {
 //Get all properties with the 
 //custom attribute [IsValueField]
 var results = new SortedList<int, PropertyInfo>();
 foreach (var prop in type.GetProperties())
 {
 var isValueField = prop.GetCustomAttributes(false).OfType<IsValueFieldAttribute>().FirstOrDefault();
 if (isValueField != null)
 results.Add(isValueField.FieldIndex, prop);
 }
 properties = results.Values;
 _propertiesCache.Add(type, properties);
 }
 return properties;
 }
}

remember to hold one global instance of this class or make everything static

you have not many Message classes and convert a lot you can also create Delegates to set the properties.

create to cache it

var message = Expression.Parameter(messageType, "message");
var value = Expression.Parameter(prop.PropertyType, "value");
var body = Expression.Assign(Expression.Property(message, prop), value);
var setProperty = Expression.Lambda(body, message, value).Compile();

and use it

setProperty.DynamicInvoke(message, fieldValue);
answered Oct 20, 2014 at 11:57
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.