Search code examples
javaandroidmultithreadingrunnable

Simple threads don't execute exactly parallely


I'm starting ten threads to update ten progress bars.
It should take five seconds to complete the bars.
When starting just one thread, it completes in five seconds.
When starting all ten threads, they update asynchronously and they complete in more than five seconds.
Is it possible to achieve this task in a more efficient way so that all ten progress bars complete in five seconds?

Here's MainActivity.java

package com.google.example;

import android.support.v7.app.AppCompatActivity;
import android.os.Bundle;
import android.view.View;
import android.widget.Button;
import android.widget.ProgressBar;

import java.util.ArrayList;

public class MainActivity extends AppCompatActivity {

    ArrayList<ProgressBar> bars = new ArrayList<>();

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        for(int i = 0; i < 10; i++){
            int id = getResources().getIdentifier("b" + (i+1), "id", getPackageName());
            bars.add((ProgressBar) findViewById(id));
        }

        Button button = findViewById(R.id.button);
        button.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View view) {

                int num = 10;

                for(int i = 0; i < num; i++){
                    MyRunnable runnable = new MyRunnable(bars.get(i));
                    Thread thread = new Thread(runnable);
                    thread.start();
                }

            }
        });

    }

    class MyRunnable implements Runnable{

        ProgressBar bar;

        MyRunnable(ProgressBar b){
            bar = b;
        }

        @Override
        public void run() {
            int progress = 0;
            long previous = 0;

            while(progress < 100) {
                long time = System.currentTimeMillis();
                if (time != previous && time % 50 == 0) {
                    progress++;
                    bar.setProgress(progress);
                    previous = time;
                }
            }

        }
    }
}

And here's activity_main.xml

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout android:layout_width="match_parent"
    android:layout_height="match_parent"
    xmlns:android="http://schemas.android.com/apk/res/android"
    android:orientation="vertical">

    <Button
        android:id="@+id/button"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:text="Start"/>

    <ProgressBar
        android:id="@+id/b1"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        style="?android:attr/progressBarStyleHorizontal"/>

    <ProgressBar
        android:id="@+id/b2"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        style="?android:attr/progressBarStyleHorizontal"/>

    <ProgressBar
        android:id="@+id/b3"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        style="?android:attr/progressBarStyleHorizontal"/>

    <ProgressBar
        android:id="@+id/b4"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        style="?android:attr/progressBarStyleHorizontal"/>

    <ProgressBar
        android:id="@+id/b5"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        style="?android:attr/progressBarStyleHorizontal"/>

    <ProgressBar
        android:id="@+id/b6"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        style="?android:attr/progressBarStyleHorizontal"/>

    <ProgressBar
        android:id="@+id/b7"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        style="?android:attr/progressBarStyleHorizontal"/>

    <ProgressBar
        android:id="@+id/b8"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        style="?android:attr/progressBarStyleHorizontal"/>

    <ProgressBar
        android:id="@+id/b9"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        style="?android:attr/progressBarStyleHorizontal"/>

    <ProgressBar
        android:id="@+id/b10"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        style="?android:attr/progressBarStyleHorizontal"/>

</LinearLayout> 

Here's the output when starting one thread only

enter image description here

Here's when starting all together

enter image description here

EDIT: As suggested by @greeble31 I solved the problem by changing MyRunnable to

class MyRunnable implements Runnable{

        ProgressBar bar;

        MyRunnable(ProgressBar b){
            bar = b;
        }

        @Override
        public void run() {
            int progress = 0;
            long hack = System.currentTimeMillis();

            while(progress < 100) {
                progress = (int) ((System.currentTimeMillis() - hack) / 50);
                bar.setProgress(progress);
            }

        }
} 

This is what it looks like now.

enter image description here


Solution

  • As @DavisHerring mentioned in a comment, this is (almost entirely) due to missed millis in MyRunnable.run(). Your code assumes that the loop will "see" perfectly sequential return values from System.currentTimeMillis().

    You might have reasoned that since the loop runs full-speed, it will no doubt be able to call System.currentTimeMillis() many times each millisecond, and therefor it is unlikely any will be missed. But then, you have more threads than you have cores, so clearly some threads must spend part of their time preempted.

    Besides, on any preemptive multi-tasking operating system, there is no guarantee that any particular thread will be executing at any particular time.

    So, if a given thread ceases executing for a brief moment, it could easily miss a multiple of 50. For example, one of the threads could "see" the following return values from System.currentTimeMillis():

    1546215544594
    1546215544594
    1546215544594
    1546215544594   <-- thread is unscheduled here, causing a 23ms gap
    1546215544617
    1546215544617
    1546215544617
    1546215544617
    

    SUGGESTED FIX

    Instead of trying to catch each milli, take a time hack at the beginning. Then just make progress equal to (elapsed time) / 50:

    progress = (int) ((System.currentTimeMillis() - hack) / 50);
    bar.setProgress(progress);
    

    As an aside: System.currentTimeMillis() is not guaranteed to return a monotonically increasing result (see System.uptimeTimeMillis() for an alternative), but for your particular problem, that's probably not as big of an issue.