none
how to apply multi threading to a method RRS feed

  • Question

  • I have a program that get user int input "1" and increment it based on the amount of files in a directory then stamp that int on each file( first is 1 and so on 1++). The foreach loop go in each directory gets its files, increment the input and call the stamp method until all files are done. In this process the order is important. However multitasking ( Parallel.ForEach) does't always guarantee order, in my understanding it returns which ever thread done first and maybe also damage the i++ functionality ( correct me if I'm wrong).

    The question is how to apply multi threading in this case? i am thinking save the values of the foreach at the end, pass it to the stamping method and have the method stamp x amount of files at a time. I don't know if its possible or how to apply.

    Here is my watermark method:

          //text comes from the foreach already set.
      public void waterMark(string text, string sourcePath, string destinationPathh)
            {
                using (Bitmap bitmap = new Bitmap(sourcePath))
                {
                    int compressionTagIndex = Array.IndexOf(bitmap.PropertyIdList, 0x103);
                    PropertyItem compressionTag = bitmap.PropertyItems[compressionTagIndex];
                    byte[] com = compressionTag.Value;
                    Encoder encoder = Encoder.Compression;
                    EncoderParameters myEncoderParameters = new EncoderParameters(1);
                    EncoderParameter myEncoderParameter = new EncoderParameter(encoder, (long)EncoderValue.CompressionCCITT4);
                    myEncoderParameters.Param[0] = myEncoderParameter;
                    ImageCodecInfo myImageCodecInfo;
                    myImageCodecInfo = GetEncoderInfo("image/tiff");
    
                    Brush brush = new SolidBrush(Color.Black);
                    Font font = new Font("Arial", 50, FontStyle.Italic, GraphicsUnit.Pixel);
                    Bitmap tempBitmap = new Bitmap(bitmap.Width, bitmap.Height);
                    Graphics tempGraphics = Graphics.FromImage(tempBitmap);
                    SizeF textSize = tempGraphics.MeasureString(text, font);
                    tempBitmap = new Bitmap(bitmap.Width, bitmap.Height + (int)textSize.Height + 10);
                    tempBitmap.SetResolution(bitmap.HorizontalResolution, bitmap.VerticalResolution);
                    using (Graphics graphics = Graphics.FromImage(tempBitmap))
                    {
                        graphics.FillRectangle(Brushes.White, 0, 0, bitmap.Width, bitmap.Height + 100);
                        graphics.DrawImage(bitmap, 0, 0, bitmap.Width, bitmap.Height);
                        Point position = new Point(bitmap.Width - ((int)textSize.Width + 200), bitmap.Height + 5);
                        graphics.DrawString((text), font, brush, position);
                        if (new[] { 2, 3, 4 }.Contains(com[0]))
    
                        {
                            tempBitmap.Save(destinationPathh, myImageCodecInfo, myEncoderParameters);
                            return;
                        }
                        tempBitmap.Save(destinationPathh, ImageFormat.Tiff);
    
                    }
                }
            }
    I can provide the foreach loop if needed. Thank you in advance.

      
    • Edited by Guest1993 Tuesday, January 21, 2020 4:43 PM
    Tuesday, January 21, 2020 4:42 PM

Answers

  • Multi threaded code, by definition, guarantees no ordering. Therefore you cannot rely on some method in thread A being called before or after some method in thread B. If you need that sort of guarantee then you probably don't need multi-threading.

    When I read about your issue I don't see multi-threading as the solution. Your belief that multi-threading will "speed up" your code is incorrect in this case because the work you're talking about isn't really CPU intensive. You may see some performance increase but I doubt it is enough to justify the changes you're going to need to make. You're going to be creating GDI objects on multiple threads at once which is going to eat up your resources if unchecked. Furthermore saving back to the file system is still ultimately going to be serialized so you are IO bound as well. The only benefit you're going to likely see is the little bit of time you aren't using either of these.

    In order to call the method you mentioned you'll need to know the "number" of the file in advance. That means you'll have to enumerate the files in the directory(ies) first before you can call this method each time. Again, that is IO bound so multi-threading isn't going to change anything here. However since you'll need to enumerate all the files first you'll then be able to solve your original question of how to map each file to a unique #. You'll need to create a simple structure to handle that (e.g. source name, target name and text). Then call your helper method for each one.

    Since this appears to be triggered from the UI I would recommend that you make this higher level method async. Then your UI will trigger the process and then move on while a background thread will enumerate the files and call your method for each one. This will make the UI responsive but the overall process will still take the same amount of time. If you really wanted to subdivide from there then your higher level method (the one with the foreach) could at that point (because you created a helper struct to store the data) use Parallel.Foreach or equivalent to scatter the work out. But, again, I doubt you'll see much perf increase and likely your resource usage will go way up. 

    I also notice you're leaking some resources in your helper method so you need to go back through this code and fix the resource leaks worse otherwise you're going to have lots of issues with memory later.


    Michael Taylor http://www.michaeltaylorp3.net

    • Marked as answer by Guest1993 Wednesday, January 29, 2020 1:59 PM
    Tuesday, January 21, 2020 7:13 PM
    Moderator
  • Hi, thanks for your time. The watermark was very clean but as requirements grew it became more and more ugly. As far as multi threading I applied it to the higher lvl function that contains foreach.

    //Before
    helper.waterMark(text + helper.getformattedString(bates), file.FullName, resultDirectory + @"\" + helper.getformattedString(fileName) + ".tif");
    //After:
    
      string newFileName = helper.getformattedString(fileName);
                                string newBate = helper.getformattedString(bates);
                                Thread newThread = new Thread(() => helper.waterMark(text + newBate, file.FullName, resultDirectory + @"\" + newFileName + ".tif"));
                                newThread.Start();
                               

    The new string variables are to set values before the threads start to avoid issues such as file already exist. This improve speed by 1 minute only, I am now looking for more optimization. thanks for the help. 

    • Marked as answer by Guest1993 Wednesday, January 29, 2020 1:58 PM
    Wednesday, January 22, 2020 4:04 PM

All replies

  • Multi threaded code, by definition, guarantees no ordering. Therefore you cannot rely on some method in thread A being called before or after some method in thread B. If you need that sort of guarantee then you probably don't need multi-threading.

    When I read about your issue I don't see multi-threading as the solution. Your belief that multi-threading will "speed up" your code is incorrect in this case because the work you're talking about isn't really CPU intensive. You may see some performance increase but I doubt it is enough to justify the changes you're going to need to make. You're going to be creating GDI objects on multiple threads at once which is going to eat up your resources if unchecked. Furthermore saving back to the file system is still ultimately going to be serialized so you are IO bound as well. The only benefit you're going to likely see is the little bit of time you aren't using either of these.

    In order to call the method you mentioned you'll need to know the "number" of the file in advance. That means you'll have to enumerate the files in the directory(ies) first before you can call this method each time. Again, that is IO bound so multi-threading isn't going to change anything here. However since you'll need to enumerate all the files first you'll then be able to solve your original question of how to map each file to a unique #. You'll need to create a simple structure to handle that (e.g. source name, target name and text). Then call your helper method for each one.

    Since this appears to be triggered from the UI I would recommend that you make this higher level method async. Then your UI will trigger the process and then move on while a background thread will enumerate the files and call your method for each one. This will make the UI responsive but the overall process will still take the same amount of time. If you really wanted to subdivide from there then your higher level method (the one with the foreach) could at that point (because you created a helper struct to store the data) use Parallel.Foreach or equivalent to scatter the work out. But, again, I doubt you'll see much perf increase and likely your resource usage will go way up. 

    I also notice you're leaking some resources in your helper method so you need to go back through this code and fix the resource leaks worse otherwise you're going to have lots of issues with memory later.


    Michael Taylor http://www.michaeltaylorp3.net

    • Marked as answer by Guest1993 Wednesday, January 29, 2020 1:59 PM
    Tuesday, January 21, 2020 7:13 PM
    Moderator
  • I agree, based on testing; this method takes most of the time(70%), it stamps one image at a time, therefor i was thinking what is the way to make it stamp many at a time based on amount of files in each directory and dividing by index#. Also, this is a MVC project 

    "I also notice you're leaking some resources in your helper method so you need to go back through this code and fix the resource leaks worse otherwise you're going to have lots of issues with memory later." can you please be more precise so I can fix it

    Tuesday, January 21, 2020 7:37 PM
  • To call this method multiple times you'll have to have the higher level method enumerate the files and give them numbers as I mentioned in my previous post. Then you can use Parallel.Foreach like you had mentioned originally to scatter the work. Alternatively you could simply enumerate the results yourself using `Task.Run` and storing the task in a list so you can wait for all the work to complete.

    "can you please be more precise so I can fix it"

    Anything that implements IDisposable needs to be wrapped in a using statement. On a cursory glance: Brush, Font, Graphics, Bitmap. However be sure the resource is actually done being used before you dispose it otherwise you'll get an error later.


    Michael Taylor http://www.michaeltaylorp3.net

    Tuesday, January 21, 2020 7:44 PM
    Moderator
  • I am thinking the same way, now I need to figure out how to apply it.

    "Anything that implements IDisposable needs to be wrapped in a using statement"

    I have them wrapped in using statements!

    Tuesday, January 21, 2020 7:53 PM
  • //No using
    Brush brush = new SolidBrush(Color.Black);
    //No using
    Font font = new Font("Arial", 50, FontStyle.Italic, GraphicsUnit.Pixel);
    //No using
    Bitmap tempBitmap = new Bitmap(bitmap.Width, bitmap.Height);
    //No using
    Graphics tempGraphics = Graphics.FromImage(tempBitmap);

    You only have 2 using statements - one for the source bitmap and one for the second graphic you create off the temp bitmap. But by this point you've already leaked the original temp bitmap and graphics you created.

    //Some type to capture data
    public class WatermarkData
    {
       public int Index { get; set; }
       public string SourceFile { get; set; }
       public string DestinationFile { get; set; }
    }
    
    //Higher level function
    var files = GetFilesToEnumerate().ToArray();
    
    //Build up the watermark data
    var watermarks = files.Select((file, index) => new WatermarkData() { Index = index, SourceFile = file, DestinationFile = ... });
    
    //Now use Parallel foreach or something similar to enumerate the watermarks
    Parallel.ForEach(watermarks, WaterMark);
    
    //Your existing method posted earlier, note I Pascal cased it to avoid conflicts
    void WaterMark ( WatermarkData data )
    {
       //Normal logic goes here
    }
    
    
                     
    


    Michael Taylor http://www.michaeltaylorp3.net

    Tuesday, January 21, 2020 8:02 PM
    Moderator
  • for the first part:

     

    using (Brush brush = new SolidBrush(Color.Black)) { using (Font font = new Font("Arial", 50, FontStyle.Italic, GraphicsUnit.Pixel)) {

    //I can't add using to tempBitmap Bitmap tempBitmap = new Bitmap(bitmap.Width, bitmap.Height); using (Graphics tempGraphics = Graphics.FromImage(tempBitmap)) { SizeF textSize = tempGraphics.MeasureString(text, font); tempBitmap = new Bitmap(bitmap.Width, bitmap.Height + (int)textSize.Height + 10); tempBitmap.SetResolution(bitmap.HorizontalResolution, bitmap.VerticalResolution); using (Graphics graphics = Graphics.FromImage(tempBitmap)) { graphics.FillRectangle(Brushes.White, 0, 0, bitmap.Width, bitmap.Height + 100); graphics.DrawImage(bitmap, 0, 0, bitmap.Width, bitmap.Height); Point position = new Point(bitmap.Width - ((int)textSize.Width + 200), bitmap.Height + 5); //Point position = new Point((int)((bitmap.Width - textSize.Width) / 2), bitmap.Height + 5); graphics.DrawString((text), font, brush, position); if (new[] { 2, 3, 4 }.Contains(com[0])) { tempBitmap.Save(destinationPathh, /*ImageFormat.Tiff,*/ myImageCodecInfo, myEncoderParameters); return; } tempBitmap.Save(destinationPathh, ImageFormat.Tiff); } } } } }


    For the second part I am still working on GetFilesToEnumerate()

    Thank you again.

    Tuesday, January 21, 2020 9:30 PM
  • If you're using C# 7.1 then you can use the "shortened" using syntax to get rid of that nesting ugliness that occurs. But even then there is a lot of nesting going on. I'm thinking there is already a method floating around on the Internet to watermark an image for you but let's go with what you have and simplify it down. (Note: not tested)

    using (Bitmap bitmap = new Bitmap(sourcePath))
    using (Brush brush = new SolidBrush(Color.Black))
    using (Font font = new Font("Arial", 50, FontStyle.Italic, GraphicsUnit.Pixel))
    {
        var textSize = TextRenderer.MeasureText(text, font);
    
        var tempBitmap = new Bitmap(bitmap.Width, bitmap.Height + textSize.Height + 10);
        tempBitmap.SetResolution(bitmap.HorizontalResolution, bitmap.VerticalResolution);
        using (Graphics graphics = Graphics.FromImage(tempBitmap))
        {
            graphics.FillRectangle(Brushes.White, 0, 0, bitmap.Width, bitmap.Height + 100);
            graphics.DrawImage(bitmap, 0, 0, bitmap.Width, bitmap.Height);
            Point position = new Point(bitmap.Width - (textSize.Width + 200), bitmap.Height + 5);
    
            graphics.DrawString((text), font, brush, position);
    
            //Make this a parameter to the function so you don't waste creating an array each time
            if (new[] { 2, 3, 4 }.Contains(com[0]))
            {
                tempBitmap.Save(destinationPathh, /*ImageFormat.Tiff,*/ myImageCodecInfo, myEncoderParameters);
                return;
            }
            tempBitmap.Save(destinationPathh, ImageFormat.Tiff);
        }                
    }


    Michael Taylor http://www.michaeltaylorp3.net

    Tuesday, January 21, 2020 9:56 PM
    Moderator
  • Hi, thanks for your time. The watermark was very clean but as requirements grew it became more and more ugly. As far as multi threading I applied it to the higher lvl function that contains foreach.

    //Before
    helper.waterMark(text + helper.getformattedString(bates), file.FullName, resultDirectory + @"\" + helper.getformattedString(fileName) + ".tif");
    //After:
    
      string newFileName = helper.getformattedString(fileName);
                                string newBate = helper.getformattedString(bates);
                                Thread newThread = new Thread(() => helper.waterMark(text + newBate, file.FullName, resultDirectory + @"\" + newFileName + ".tif"));
                                newThread.Start();
                               

    The new string variables are to set values before the threads start to avoid issues such as file already exist. This improve speed by 1 minute only, I am now looking for more optimization. thanks for the help. 

    • Marked as answer by Guest1993 Wednesday, January 29, 2020 1:58 PM
    Wednesday, January 22, 2020 4:04 PM
  • Hi Guest1993,

    If the reply is helpful for you, could you please mark it as answer, it would be helpful for other communities who has similar issue. 

    Best regards,

    Zhanglong


    MSDN Community Support
    Please remember to click "Mark as Answer" the responses that resolved your issue, and to click "Unmark as Answer" if not. This can be beneficial to other community members reading this thread. If you have any compliments or complaints to MSDN Support, feel free to contact MSDNFSF@microsoft.com.

    Wednesday, January 29, 2020 1:57 AM
    Moderator