1
\$\begingroup\$

I am writing code to update data in the sample table (i.e. stbl) atoms of a video/audio file and found that in my method UpdateValues slows down to almost a crawl when executed. The line values.Add(value) was determined to be where the greatest time was taken through the debugger. Stepping through the program it was determined to take ~10mS to process. This List<int> is not touched by any other portion of the program. No other code appears to affect this list during the process.

public byte[] UpdateValues(int offset, string varName)
{
    //Create initial list
    List<int> values = new List<int>();
    //Set initial position to offset from (size, name, flags, count)
    int position = ((varName == "stsz") ? 20 : 16);
    //temporary byte array
    byte[] tempSize = new byte[4];

    //continue until position is equal to length of data
    while (position < this.data.Length)
    {
        //Get value from position
        Array.Copy(this.data, position, tempSize, 0, 4);

        if (BitConverter.IsLittleEndian)//Check for Endianess
            Array.Reverse(tempSize);//Reverse if necessary

        //update the value to include the new offset
        int value = BitConverter.ToInt32(tempSize, 0) + offset;

        //Add the new value to a list
        values.Add(value);//fixed from 'Values', no change in output time
        //increment the location
        position += 4;
    }

    //Sets the values back to the original location as updated
    return SetValues(values, varName);
}

The SetValues method is not slow and takes less than a second to complete:

public byte[] SetValues( List<int> values, string name )
{
    int location = 0;

    int _size = (values.Count * 4) + ((name == "stsz")? 20 : 16);
    this.data = new byte[_size];
    byte[] temp = BitConverter.GetBytes(_size);
    if (BitConverter.IsLittleEndian)
        Array.Reverse(temp);

    //add size into first 4
    Array.Copy(temp, 0, this.data, location, temp.Length);
    location += temp.Length;

    //add box type label
    temp = Encoding.ASCII.GetBytes(name);
    Array.Copy(temp, 0, this.data, location, temp.Length);
    location += temp.Length;

    //add FLAGS
    temp = new byte[(name == "stsz")? 8 : 4];//add 4 bytes of 0 flags and version
    Array.Copy(temp, 0, this.data, location, temp.Length);
    location += temp.Length;

    //add Entry Count
    temp = BitConverter.GetBytes(values.Count);
    if (BitConverter.IsLittleEndian)
        Array.Reverse(temp);
    Array.Copy(temp, 0, this.data, location, temp.Length);
    location += temp.Length;



    //Add list of offsets to total
    foreach (int a in values)
    {
        temp = BitConverter.GetBytes(a);
        if (BitConverter.IsLittleEndian)
            Array.Reverse(temp);
        Array.Copy(temp, 0, this.data, location, temp.Length);
        location += temp.Length;
    }
    return this.data;
}
\$\endgroup\$

closed as unclear what you're asking by paparazzo, Sᴀᴍ Onᴇᴌᴀ, Vogel612, Snowbody, Mast Dec 30 '17 at 19:22

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 3
    \$\begingroup\$ Just tested these 2 methods with 300000 bytes. In debug mode they finished in 19ms. I guess your problem lies somewhere else. \$\endgroup\$ – Heslacher Dec 28 '17 at 15:27
  • 1
    \$\begingroup\$ Something else has to be going on. I should not take 10 ms to add a value to values. \$\endgroup\$ – paparazzo Dec 28 '17 at 16:43
  • 1
    \$\begingroup\$ Could you please add more information as to how you determined that the values.Add(value) is the source of the slowdown? Is there other code that surrounds this that modifies values? \$\endgroup\$ – Snowbody Dec 29 '17 at 18:15
  • 1
    \$\begingroup\$ @Snowbody I have updated my question with the information you requested. I have also already answered my question by rewriting the method to reduce unnecessary use of the list. \$\endgroup\$ – vipersassassin Jan 1 '18 at 4:19
  • 1
    \$\begingroup\$ BTW, using the debugger to time statements is not reliable. \$\endgroup\$ – Snowbody Jan 1 '18 at 5:03
2
\$\begingroup\$

There are several Comments that are unnecessary. You should get rid of them, since your code explains a few of those Comments.

Most C# coding standards suggest that you always use braces for if statements, so instead of this:

 if (BitConverter.IsLittleEndian)//Check for Endianess
        Array.Reverse(tempSize);//Reverse if necessary

you would format it like this:

if (BitConverter.IsLittleEndian)
{
    Array.Reverse(tempSize);
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Most comments were added solely for posting to this site. Original code only had 3 comments. I would agree with the braces for the if statements, this was low level code that had not been completed and was being tested. I appreciate the response. \$\endgroup\$ – vipersassassin Dec 28 '17 at 20:16
  • 1
    \$\begingroup\$ @vipersassassin Comments should add value to the code by explaining things that are non-obvious, especially WHY the code is doing what it is doing. Comments like "temporary byte array" are obvious from context; the comment should explain what the byte array is used for and why it's needed. \$\endgroup\$ – Snowbody Jan 2 '18 at 14:10
1
\$\begingroup\$

Due to the addition of int to values being the point of slow down, code was adjusted and properly fitted.

The method UpdateValues creates a List<int> that is later written over the data in the same location through the method SetValues. Removing the List<int> and simply changing the values in-line with the location it was originally taken from removes the need for the extra call, as well as removes the need for the List<int> in the method UpdateValues.

    public byte[] UpdateValues( int offset, string varName )
    {

        int position = ((varName == "stsz") ? 20 : 16);

        byte[] tempSize = new byte[4];

        //continue until position is equal to length of data
        while (position < this.data.Length)
        {
            //Get value from position
            Array.Copy(this.data, position, tempSize, 0, 4);

            if (BitConverter.IsLittleEndian)
                Array.Reverse(tempSize);

            //update the value with the offset given
            int value = BitConverter.ToInt32(tempSize, 0) + offset;

            tempSize = BitConverter.GetBytes(value);
            if (BitConverter.IsLittleEndian)
                Array.Reverse(tempSize);

            //write bytes back to same location as read from
            Array.Copy(tempSize, 0, this.data, position, tempSize.Length);

            //Move forward to next size
            position += 4;
        }

        //return the updated data
        return this.data;
    }
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.