Насколько верна такая реализация многопоточности?

#c_sharp #net #многопоточность #инспекция_кода #net_40

Не знаю, корректно ли на этом сайте такие вопросы задавать... В общем я не уверен
что код ниже верно написан. Свою задачу он выполняет, проверял неоднократно. Однако
меня гложут сомнения в том, насколько эта реализация может считаться правильной. Быть
может на C# такого рода задачу принято реализовывать иначе? Хотелось бы получить критику
и разъяснения.

Сама программа - аудио конвертер. Ищет в папке подходящие файлы и запускает внешний,
консольный конвертер для их преобразования в wav.

public partial class Form1 : Form
{
    static object locker = new object();
    string[] findFiles;
    int fifi = -1;

    public Form1()
    {
        InitializeComponent();
        Conv();
    {

    async void Conv()
    {
        findFiles = Directory.GetFiles(Application.StartupPath, "*.ape", SearchOption.AllDirectories).Union(Directory.GetFiles(Application.StartupPath,
"*.ogg", SearchOption.AllDirectories)).ToArray();

        // Поддерживаем не более 8 ядер.
        int p;
        if (Environment.ProcessorCount < 8)
            p = Environment.ProcessorCount;
        else
            p = 8;

        int t;
        // Если количество файлов больше чем ядер, то устанавливаем столько потоков,
сколько ядер.
        if(findFiles.Length > p)
            t = p;
        else // Если файлов меньше чем ядер, то потоков устанавливаем сколько файлов.
            t = findFiles.Length;

        // Устанавливаем количество потоков.
        Task[] tasks = new Task[t];

        // Запускаем потоки.
        for (int i = 0; i <= p - 1; i++)
        {
            if(i <= findFiles.Length - 1)
            {
                tasks[i] = new Task(() => ConvertDoWork(++fifi));
                tasks[i].Start();
            }
        }

        // Ждём выполнения всех задач.
        await TaskEx.WhenAll(tasks);

        Application.Exit();
    }

    int p, pc;
    public void ConvertDoWork(int num)
    {
        int n = num;

        if ((findFiles.Length == 0) || (n > findFiles.Length))
            return;

        string s = findFiles[n];

        ProcessStartInfo startInfo = new ProcessStartInfo();
        if (Path.GetExtension(s).Equals(".ogg"))
        {
            startInfo.FileName = "oggdec.exe";
            startInfo.Arguments = "-Q \"" + s + "\"";
        }
        else
            if (Path.GetExtension(s).Equals(".ape"))
        {
            startInfo.FileName = "MAC.exe";
            startInfo.Arguments = "\"" + s + "\" \"" + Path.ChangeExtension(s, ".wav")
+ "\" -d";
        }
        startInfo.WorkingDirectory = Application.StartupPath;
        startInfo.CreateNoWindow = true;

        Process processReg = new Process();
        processReg.StartInfo = startInfo;
        processReg.StartInfo.WindowStyle = ProcessWindowStyle.Hidden;
        processReg.Start();
        processReg.WaitForExit();

        lock (locker)
        File.Delete(s);

        int percentComplete = (int)Math.Round((double)(100 * n) / findFiles.Length);
        pc = percentComplete / 10;

        try
        {
            this.Invoke((MethodInvoker)delegate
            {
                progressBar1.Value = percentComplete;

                // Если не выбрано не отображать прогресс, то отображаем всплывашку.
                if ((notifyIcon1.Visible) && (!chkNotNotify.Checked) && (!pc.Equals(p)))
                {
                    notifyIcon1.ShowBalloonTip(300, "Прогресс", "Выполнено " + percentComplete
+ "% работы", ToolTipIcon.Info);
                    p = pc;
                }

            });

        }
        catch (System.Exception ex)
        {

        }

        lock (locker)
        ++fifi;
        if (fifi < findFiles.Length)
            ConvertDoWork(fifi);
    }

    

Ответы

Ответ 1

1.

Слишком много переменных уровня класса. Каждую из них надо отдельно перепроверять
- а мне лень. Куда безопаснее локальные переменные и параметры функции.

2.

Переменная fifi используется опасным образом. Лучше от нее вовсе отказаться, а распараллеливание
делать другими методами.

Для распараллеливания через задачи я сам использую и всем рекомендую вот такую конструкцию:

await TaskEx.WhenAll(
   from file in findFiles
   select Task.Run(() => ConvertDoWork(file))
);


Вам, я так понимаю, нужно еще ограничить число число одновременно выполняемых задач
числом ядер. Вручную это можно сделать вот так:

class LimitedConcurrencyGuard {
  private readonly object _lock = new object();
  private readonly Queue> queue = new Queue>();
  private int slots;

  public LimitedConcurrencyGuard (int slots) { this.slots = slots; }

  public async Task AcquireSlot() {
    TaskCompletionSource