2
\$\begingroup\$

I was trying to read data from a file and after processing data write it to another file. Reading and writing do not need much CPU time, so I think it would be better if I use multithreading and reduce running time.

The code uses backgroundworker for showing process state and in DoWork it uses multithreading. I think it can be improved in many ways. For example, it is currently possible that, when a thread is reading/writing, another thread will try to write/read on another file, and because of round robin, hard head will have to move several times.

public class ReaderWriter
    {
        //variables
        private string fileToRead = "";
        private string fileToWrite = "";
        private long fileToReadOffSet = 0;
        private long fileToWriteOffSet = 0;
        private long readWriteLength = 0;
        private int readBufferLength = 0;
        private byte[] readBuffer = new byte[0];
        private byte[] writeBuffer = new byte[0];
        private bool cancel = false;
        private bool readBufferIsValid = false;
        private bool writeBufferIsValid = false;
        private readonly object readBufferLock = new object();
        private readonly object writeBufferLock = new object();
        //===============================================================
        //encapsulate(also manage multithreading)
        public bool Cancel
        {
            get { return cancel; }

            set
            {
                if (value == true)
                {
                    cancel = value;
                }
            }
        }

        public byte[] ReadBuffer
        {
            get
            {
                lock (readBufferLock)
                {
                    if (!readBufferIsValid)
                    {
                        Monitor.Wait(readBufferLock);
                    }

                    //get
                    byte[] result = (byte[])readBuffer.Clone();

                    //data readed and current data is not valid
                    readBufferIsValid = false;

                    //pulse
                    Monitor.Pulse(readBufferLock);

                    return result;
                }
            }

            set
            {
                lock (readBufferLock)
                {
                    if (readBufferIsValid)
                    {
                        Monitor.Wait(readBufferLock);
                    }

                    //set
                    readBuffer = (byte[])value.Clone();

                    //current data is valid
                    readBufferIsValid = true;

                    //pulse
                    Monitor.Pulse(readBufferLock);
                }
            }
        }

        public byte[] WriteBuffer
        {
            get
            {
                lock (writeBufferLock)
                {
                    if (!writeBufferIsValid)
                    {
                        Monitor.Wait(writeBufferLock);
                    }

                    //get
                    byte[] result = (byte[])writeBuffer.Clone();

                    //data readed and current data is not valid
                    writeBufferIsValid = false;

                    //pulse
                    Monitor.Pulse(writeBufferLock);

                    return result;
                }
            }

            set
            {
                lock (writeBufferLock)
                {
                    if (writeBufferIsValid)
                    {
                        Monitor.Wait(writeBufferLock);
                    }

                    //set
                    writeBuffer = (byte[])value.Clone();

                    //current data is valid
                    writeBufferIsValid = true;

                    //pulse
                    Monitor.Pulse(writeBufferLock);
                }
            }
        }
        //===============================================================
        //constructor
        public ReaderWriter( string fileForReadFrom, string fileForWriteTo, long readFileStartOffSet, long writeFileStarOffSet, long readWriteSize, int bufferSize)
        {
            fileToRead = fileForReadFrom;
            fileToWrite = fileForWriteTo;
            fileToReadOffSet = readFileStartOffSet;
            fileToWriteOffSet = writeFileStarOffSet;
            readWriteLength = readWriteSize;
            readBufferLength = bufferSize;
        }
        //===============================================================
        //methods
        public void Read()
        {
            using (FileStream streamReader = new FileStream(fileToRead, FileMode.Open))
            using (BinaryReader binaryReader = new BinaryReader(streamReader))
            {
                //seek
                streamReader.Seek(fileToReadOffSet, SeekOrigin.Begin);

                //read
                while (!cancel && streamReader.Position < readWriteLength + fileToReadOffSet)
                {
                    if (streamReader.Position + readBufferLength > readWriteLength + fileToReadOffSet)
                    {
                        readBufferLength = Convert.ToInt32(readWriteLength + fileToReadOffSet - streamReader.Position);
                    }

                    ReadBuffer = binaryReader.ReadBytes(readBufferLength);
                }
            }
        }

        public void Write()
        {
            using (FileStream streamWriter = new FileStream(fileToWrite, FileMode.Create))
            using (BinaryWriter binaryWriter = new BinaryWriter(streamWriter))
            {
                streamWriter.Seek(fileToWriteOffSet, SeekOrigin.Begin);

                //write
                while (!cancel && streamWriter.Position < fileToWriteOffSet + readWriteLength)
                {
                    binaryWriter.Write(WriteBuffer);
                }
            }
        }
    }

Proc class that will do what it needs:

public class Proc
        {
            //variables
            ReaderWriter readerWriter;
            long dataLength;
            long processedLength = 0;
            bool cancel = false;
            int oldprecentage = 0;
            BackgroundWorker worker = null;
            //================================================
            //constructor
            public Proc(ReaderWriter ReaderWriterInstance, BackgroundWorker backgroundWorker, long dataSize)
            {
                readerWriter = ReaderWriterInstance;
                dataLength = dataSize;
                worker = backgroundWorker;
            }
            //================================================
            //methods
            public void Do()
            {
                while (processedLength < dataLength)
                {
                    //read
                    byte[] data = readerWriter.ReadBuffer;

                    //do
                    //...Do Whatever Needs...

                    //write
                    readerWriter.WriteBuffer = data;

                    //update processedLength
                    processedLength += data.Length;

                    //for reporting progress
                    if (((int)(processedLength * 100 / dataLength)) > oldprecentage)
                    {
                        oldprecentage = (int)(processedLength * 100 / dataLength);
                        worker.ReportProgress(oldprecentage);
                    }

                    //manage cancel
                    if (worker.CancellationPending)
                    {
                        cancel = true;
                        readerWriter.Cancel = true;
                        break;
                    }
                }

                //if cancel then cancel readerWriter also
                if (cancel)
                {
                    byte[] fakeData = new byte[0];
                    fakeData = readerWriter.ReadBuffer;//for unlock if it is locked

                    fakeData = new byte[0];
                    readerWriter.WriteBuffer = fakeData;//for unlock if it is locked
                }
            }
        }

DoWork:

    void backgroundWorker_DoWork(object sender, DoWorkEventArgs e)
            {
                FileInfo fileInfo = new FileInfo(file1);
                int bufferSize = 4 * 1024 * 1024;

                ReaderWriter readWrite = new ReaderWriter(file1, file2, 0, 0, fileInfo.Length, bufferSize);
                Proc process = new Proc(readWrite, backgroundWorker, fileInfo.Length);

                //read-proc-write
                Task[] tasks = new Task[3];
                var parentTask = Task.Run(() =>
                {

 tasks[0] = Task.Factory.StartNew(() =>
                            {
                                process.Do();
                            });
 tasks[1] = Task.Factory.StartNew(() =>
                            {
                                readWrite.Read();
                            });
tasks[2] = Task.Factory.StartNew(() =>
                            {
                                readWrite.Write();
                            });
                });

                //wait untill evething be done
                parentTask.Wait();
                Task.WaitAll(tasks);
            }
\$\endgroup\$
5
  • 1
    \$\begingroup\$ You use BackgroundWorker AND Task (TPL) at the same time? Why would you do that? \$\endgroup\$
    – t3chb0t
    Commented Feb 8, 2017 at 7:51
  • 3
    \$\begingroup\$ You are over-engineering to the point that it is hard to give a review that won't turn into complete rewrite. If I were you, I would use async API to do I/O: msdn.microsoft.com/en-us/library/kztecsys(v=vs.110).aspx . \$\endgroup\$
    – Nikita B
    Commented Feb 8, 2017 at 8:49
  • \$\begingroup\$ @t3chb0t there is more than this codes in backgroundworker and only this part needs more threads.Nikita B ,thanks I will look at it. \$\endgroup\$
    – NokhodSiah
    Commented Feb 10, 2017 at 13:15
  • \$\begingroup\$ @NikitaB thank for response , but I really can not see how it may help. I need read-manipulate-write a buffer, by using async I should await until it be read,await until it be manipulated and then write it. I think it is not asynchronous. or maybe I missed something. \$\endgroup\$
    – NokhodSiah
    Commented Feb 10, 2017 at 17:40
  • \$\begingroup\$ @NokhodSiah An await expression does not block the thread on which it is executing. Instead, it causes the compiler to sign up the rest of the async method as a continuation on the awaited task. Control then returns to the caller of the async method. When the task completes, it invokes its continuation, and execution of the async method resumes where it left off. more at: msdn.microsoft.com/en-us/library/hh156528.aspx \$\endgroup\$
    – Nikita B
    Commented Feb 10, 2017 at 22:34

1 Answer 1

1
\$\begingroup\$

As you are doing, I have also used threads with Sockets but this technic is nor recommendable because the I/O operations could be affected, getting blocked, each time that threads change its status (running to suspend).

As @Nikita said, using async methods will improve the performance. C# provide Streams that offer async methods to work with an asynchronous designe, for example:

  • For Network operations you can use NetworkStream.
  • For Files operations you can use FileStream.

Both of them provide the methods:

  • BegineWrite.
  • EndWrite.
  • BeginRead.
  • EndRead.

With these methods you can designe your program into a single thread and am asyncrhonous behaivor, avoiding locking operations into sockets or file and the program`s performance will be increased.

Let me share you an answer of mine that talk about this issue:

https://stackoverflow.com/questions/43327534/c-sharp-high-cpu-usage-on-listener-thread-sleeping-misses-disconnect/43332998#43332998

\$\endgroup\$

Your Answer

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

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