Multithreaded printing from ArrayList

user_4007683 0
Viewed 62

I am trying to have the logger print all the log messages from the application to the console and in the future an external file. It should do this when I fire the function: "dumpToConsole". It should be done multithreaded through 3 threads that all 3 access the CopyOnWriteArrayList. The problem is that the output is not in order, and triple of what it should be. Instead of 3 messages I get 9. I need the 3 threads to all individual print 1 for example instead of each thread printing 3.

See below for my actual implementation of this.

My thread: 

public class LoggingThread extends Thread {
private boolean isStopped = false;
private CopyOnWriteArrayList<LogMessage> messages;

public LoggingThread(CopyOnWriteArrayList messages) {
    this.messages = messages;
}

@Override
public void run() {
    for (int i = 0; i < messages.size(); i++) {
        writeMessageToConsole(messages.get(i).getMessageText(), messages.get(i).getLogLevel());
    }
}

private synchronized void writeMessageToConsole(String message, LogLevel logLevel) {
    System.out.println(message + " (" + logLevel + ")");
}
}

My Logger:

public class Logger implements ILogger {
private static ILogger instance = null;
private CopyOnWriteArrayList<LogMessage> messages = new CopyOnWriteArrayList<LogMessage>();
private LoggingThread thread1;
private LoggingThread thread2;
private LoggingThread thread3;

public static ILogger getInstance() {
    if (instance == null) {
        instance = new Logger();
    }

    return instance;
}

public CopyOnWriteArrayList<LogMessage> getMessages() {
    return messages;
}

public void log(Exception ex) {
    log(ex.getMessage(), LogLevel.FATAL);
}

public void log(String message, LogLevel logLevel) {
    messages.add(new LogMessage(message, logLevel));
}

public LogMessage getLastLog() {
    if(!messages.isEmpty()) {
        return messages.get(messages.size() -1);
    }

    else {
        return new LogMessage("", LogLevel.DEBUG);
    }
}

public void dumpToConsole() {
    log("TEST1", LogLevel.FATAL);
    log("TEST2", LogLevel.DEBUG);
    log("TEST3", LogLevel.FATAL);

    thread1 = new LoggingThread(this.messages);
    thread2 = new LoggingThread(this.messages);
    thread3 = new LoggingThread(this.messages);

    thread1.start();
    thread2.start();
    thread3.start();

    try {
        thread1.join();
        thread2.join();
        thread3.join();
    }

    catch (InterruptedException e) {
        log(e.getMessage(), LogLevel.FATAL);
    }

    thread1.interrupt();
    thread2.interrupt();
    thread3.interrupt();
}
}

And my message class:

public class LogMessage {
private String messageText;
private LogLevel logLevel;

public LogMessage(String messageText, LogLevel logLevel) {
    this.messageText = messageText;
    this.logLevel = logLevel;
}

public LogLevel getLogLevel() {
    return logLevel;
}

public String getMessageText() {
    return messageText;
}
}

And the result is:

TEST1 (FATAL)
TEST2 (DEBUG)
TEST3 (FATAL)
TEST1 (FATAL)
TEST1 (FATAL)
TEST2 (DEBUG)
TEST3 (FATAL)
TEST2 (DEBUG)
TEST3 (FATAL)
thread-safety threadpool logging multithreading java
2 Answers

Each thread should get a different task to run. You implemented run() so the thread would go over all the messages instead of letting each thread bulk of messages.

Give each thread the indices it should write to console. Assuming you want to create 3 threads and you have 12 messages, then: Thread 1 will print 0 to 3, Thread 2 will print 4 to 7, Thread 3 will print 8 to 11 

public class LoggingThread extends Thread {
private boolean isStopped = false;
private CopyOnWriteArrayList<LogMessage> messages;
private int start,end;

public LoggingThread(CopyOnWriteArrayList messages, int start, int end) {
    this.messages = messages;
    this.start=start;
    this.end=end;
}

@Override
public void run() {
    for (int i = start; i < messages.size() && i<end; i++) {
        writeMessageToConsole(messages.get(i).getMessageText(), messages.get(i).getLogLevel());
    }
}

private synchronized void writeMessageToConsole(String message, LogLevel logLevel) {
    System.out.println(message + " (" + logLevel + ")");
}
}

And create your logs:

thread1 = new LoggingThread(this.messages, 0, this.messages.length()/3);
thread2 = new LoggingThread(this.messages, this.messages/3, 2*(this.messages.length()/3));
thread3 = new LoggingThread(this.messages, 2*(this.messages.length()/3), this.messages.length());

Each thread got third of the messages to print.

About the order... well, the thing about threads is that they are running simultaneously, so you can't really know which will finish first and when each log is gonna be printed.

You can, however, wait for each thread to finish - before starting the next. this will keep the logs in order:

   thread1.start();
   thread1.join();
   thread2.start();
   thread2.join();
   thread3.start();
   thread3.join();

But then there is no point for using three threads.

user_57500160

A distinct non-answer: forget about using 3 threads here.

The problem is that the output is not in order, and triple of what it should be. Instead of 3 messages I get 9.

Sure. Because you ask three 3 threads to do the work that a single thread should be doing, and each one does the same work again.

So, first of all: as soon as you say "multi threading", and you talk about printing list content, then all bets regarding ordering are off. Of course, each thread will print the messages in the correct order, but you have no control whether T1 prints all messages first, or just one, then 3 from T2, whatever. The essence of "un sync'ed" threads is that: undefined order.

Then: even when you add the necessary steps to somehow "sync up" your threads, you gain nothing from doing so.

There is one list in memory. There is one output file (or console). Using multiple threads to fetch the values and put them into your file doesn't speed up anything! It only creates overhead, and as seen: it creates the need for a lot "sync" code.

user_15311240
Related