Подсчет запятых в тексте (многопоточность), я делаю это правильно?

Question

Я подсчитываю запятые в тексте, используя 5 потоков, разбивая текст на 5 равных частей и позволяя каждой нити работать на своей собственной части. Я просто интересно, если я делаю это правильно:

public class CommaCounter implements Runnable { 

    public static int commaCount = 0; // it's static so that all CommaCounter threads share the same variable 

    private String text; 
    private int startIndex; 
    private int endIndex; 

    public CommaCounter(String text, int startIndex, int endIndex) { 
     this.text = text; 
     this.startIndex = startIndex; 
     this.endIndex = endIndex; 
    } 

    @Override 
    public void run() { 

     for (int i = startIndex; i < endIndex; i++) { 
      if(text.charAt(i) == ','){ 
       commaCount++; // is incrementing OKAY? is there danger of thread interference or memory consistency errors? 
      } 
     } 
    } 
} 

И главный метод:

public class Demo { 

    public static void main(String[] args) throws MalformedURLException, IOException, InterruptedException { 

     long startTime = System.currentTimeMillis(); 
     /* 
      I'll spare the code that was here for retrieving the text from a URL 
     */ 

     String text = stringBuilder.toString(); 

     Set<Thread> threadCollection = new HashSet<>(); 

     int threadCount = 5; 
     int textPerThread = text.length()/threadCount; 
     for (int i = 0; i < threadCount; i++) { 
      int start = i * textPerThread; 
      Thread t = new Thread(new CommaCounter(text, start, start + textPerThread)); 
      threadCollection.add(t); 
      t.start(); 
     } 

     for (Thread thread : threadCollection) { 
      thread.join(); // joining each CommaCounter thread, so that the code after the for loop doesn't execute prematurely 
     } 

     long endTime = System.currentTimeMillis(); 
     System.out.println("Counting the commas with " + threadCount + " threads took: " + (endTime - startTime) + "ms"); 
     System.out.println("Comma count: " + CommaCounter.commaCount); 
    } 

} 

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

Любая помощь будет оценена!

Answer 1

Абсолютно не в порядке. Вы получаете доступ к статической переменной из нескольких потоков. Используйте AtomicInteger или синхронизированный статический метод.

Как указано в свой комментарий, точно :)

Либо делают его AtomicInteger и использовать метод getAndIncrement или incrementAndGet,

или создать статический синхронизированный метод,

или создать синхронизированный блок , но в этом случае убедитесь, что объект, по которому вы синхронизируете, является одним и тем же! Поскольку речь идет о статической переменной в классе CommaCounter, что может быть CommaCounter.class

Answer 2

Я думаю, эта проблема точно соответствовать вилы и присоединиться рамки

https://docs.oracle.com/javase/tutorial/essential/concurrency/forkjoin.html

Поскольку она использует параллельную разделяй и властвуй технику, будет продолжать разделять текст и назначать работу новому потоку, также используя технику «рабочего стелса», чтобы гарантировать, что все потоки заняты.