Read file while updating progress bar with coroutines

Question

I'm trying to read a file line by line while I update a progress bar (two GUI textures with a float expansion in one of its width (maxWidth * currentPercentage)).

I have two implementations:

     public static string ThreadedFileRead(string path, Action<float> percAction)
     {
         FileInfo fileInfo = new FileInfo(path);
         StringBuilder sb = new StringBuilder();

         float length = fileInfo.Length;
         int currentLength = 0;

         using (StreamReader sr = new StreamReader(path))
         {
             while (!sr.EndOfStream)
             {
                 string str = sr.ReadLine();
                 sb.AppendLine(str);

                 // yield return str;

                 percAction(currentLength / length);
                 currentLength += str.Length;
                 Interlocked.Add(ref currentLength, str.Length);
             }

             percAction(1f);

             return sb.ToString();
         }
     }

Using the following implementation:

   // Inside a MonoBehaviour

   public void Start() 
   {
        string fileContents = "";
        StartCoroutine(LoadFileAsync(Application.dataPath + "/Data/file.txt", (s) => fileContents = s));
   }

   public IEnumerator LoadFileAsync(string path, Action<string> fin) 
   {
         string contents = "";

         lock (contents)
         {
             var task = Task.Factory.StartNew(() =>
             {
                 contents = F.ThreadedFileRead(path, (f) => currentLoadProgress = f);
             });

             while (!task.IsCompleted)
                 yield return new WaitForEndOfFrame();

             fin?.Invoke(contents);
         }
   }

But this blocks the current GUI (I don't know why).

I also used this:

     // Thanks to: https://stackoverflow.com/questions/41296957/wait-while-file-load-in-unity
     // Thanks to: https://stackoverflow.com/a/34378847/3286975
     [MustBeReviewed]
     public static IEnumerator LoadFileAsync(string pathOrUrl, Action<float> updatePerc, Action<string> finishedReading)
     {
         FileInfo fileInfo = new FileInfo(pathOrUrl);

         float length = fileInfo.Length;

         // Application.isEditor && ??? // Must review
         if (Path.IsPathRooted(pathOrUrl))
             pathOrUrl = "file:///" + pathOrUrl;

         /*

         using (var www = new UnityWebRequest(pathOrUrl))
         {
             www.downloadHandler = new DownloadHandlerBuffer();

             CityBenchmarkData.StartBenchmark(CityBenchmark.SendWebRequest);

             yield return www.SendWebRequest();

             CityBenchmarkData.StopBenchmark(CityBenchmark.SendWebRequest);

             while (!www.isDone)
             {
                 // www.downloadProgress
                 updatePerc?.Invoke(www.downloadedBytes / length); // currentLength / length
                 yield return new WaitForEndOfFrame();
             }

             finishedReading?.Invoke(www.downloadHandler.text);
         }

          */

         using (var www = new WWW(pathOrUrl))
         {
             while (!www.isDone)
             {
                 // www.downloadProgress
                 updatePerc?.Invoke(www.bytesDownloaded / length); // currentLength / length
                 yield return new WaitForEndOfFrame();
             }

             finishedReading?.Invoke(www.text);
         }
     }

With the following implementation:

   public IEnumerator LoadFileAsync(string path, Action<string> fin) 
   {
        yield return F.LoadFileAsync(path, (f) => currentLoadProgress = f, fin);
   }

The last code I shared has two parts:

The commented part blocks also the main thread.
The WWW class I used (it will be deprecated in a future) doesn't block the main thread, but it only displays two steps on the progress bar (like 25% and 70%).

I don't why this is happening and if there is a better approach for this.

So, any help (guidance) for this is welcome.

Answer 1

Wow, the amount of bad habits and errors in those code examples is kinda scary ^^.

Lets start with the "ThreadedFileRead" method. The first thing that could cause issues are those two lines:

 string str = sr.ReadLine();
 sb.AppendLine(str);

I'll assume that you actually read text files, otherwise using ReadLine wouldn't make any sense. The problem here is that ReadLine will remove the line seperation character(s) and AppendLine will add line seperation character(s). Depending on the platform that could be one or two characters. If you're on windows it generally uses two (0x0D + 0x0A == "\r\n"). If the original text only contains line feed characters the resulting text will be longer. On the other hand on unix / mac / linux / android we usually have only one character (most the time just a line feed "\n", though mac also uses just a carriage return "\r").

This leads to the next issue that incrementing the total byte count by the string length will actually miss the line seperation characters which are of course included in the file size. So you will never reach the actual filesize.

The next strange thing are those two lines:

 currentLength += str.Length;
 Interlocked.Add(ref currentLength, str.Length);

Both will increment currentLength by the str.Length while only one should be used. Using Interlocked add on a local variable that is only used locally makes no sense.

About the second code block, i guess the Start method is not the actual code since using a local string variable in a lambda makes little sense since the variable is out of scope when the thread finishes executing. Unless there's another closure that closes over the same local variable for later use this just looks wrong.

Next thing is the lock inside your coroutine. First of all it's just pointless. A lock only makes sense if another thread may lock on the same object and only one thread can execute at that time. The next related issue is that you should generally not use strings as lock objects for several reasons. In this case you actually lock on the empty string object. This is the same across all empty strings since it's an interned value. Keep in mind that locks work on the object value, not on variables. Finally a lock that spans over the whole procedure simply enforces synchonous execution if another thread also uses the same lock object. You should aquire a lock only when necessary and hold it as short as possible. Using locks for a long time makes multithreading pretty pointless.

The next issue may be "Task.Factory.StartNew". This topic is a bit too complex to handle here, but you may want to read this blog carefully

Next, don't use "WaitForEndOfFrame" unless you really need it (and that's almost never the case). The only cases where you actually want to use it is when you want to do additional rendering at the end of a frame after everything is done. Creating a "WaitForEndOfFrame" object requires memory. If you just want to run each frame, return "null".

About your third code block. The commented out code will not produce any increasing percentage as you yield on "www.SendWebRequest();". this will pause the coroutine until the download is completed

The threaded approach should work, though i wouldn't recommend using "Task.Factory.StartNew" but simply "Task.Run". Note that nowadays even quite large files are loaded within a fraction of a second. Reading a large file line by line will actually make the speed worse, a lot. That's due to the large amount of memory you're allocating and all the internal buffer copying that you're stringbuilder has to perform. This can be avoided by setting the capacity at the beginning:

 float length = fileInfo.Length;
 StringBuilder sb = new StringBuilder(length);

Though still reading a file line by line is always slower than reading the whole file at once.

Read file while updating progress bar with coroutines

1 Reply

Your answer

Follow this Question

Related Questions