CODEWITHSUNDEEP
javamultithreadingparsing
Alex
Alex

Reputation: 887

Java multithreaded parser

I am writing a multithreaded parser. Parser class is as follows.

public class Parser extends HTMLEditorKit.ParserCallback implements Runnable {

    private static List<Station> itemList = Collections.synchronizedList(new ArrayList<Item>());
    private boolean h2Tag = false;
    private int count;
    private static int threadCount = 0;

    public static List<Item> parse() {
        for (int i = 1; i <= 1000; i++) { //1000 of the same type of pages that need to parse

            while (threadCount == 20) { //limit the number of simultaneous threads
                try {
                    Thread.sleep(50);
                } catch (InterruptedException ex) {
                    ex.printStackTrace();
                }
            }

            Thread thread = new Thread(new Parser());
            thread.setName(Integer.toString(i));
            threadCount++; //increase the number of working threads
            thread.start();            
        }

        return itemList;
    }

    public void run() {
        //Here is a piece of code responsible for creating links based on
        //the thread name and passed as a parameter remained i,
        //connection, start parsing, etc.        
        //In general, nothing special. Therefore, I won't paste it here.

        threadCount--; //reduce the number of running threads when current stops
    }

    private static void addItem(Item item) {
        itenList.add(item);
    }

    //This method retrieves the necessary information after the H2 tag is detected
    @Override
    public void handleText(char[] data, int pos) {
        if (h2Tag) {
            String itemName = new String(data).trim();

        //Item - the item on which we receive information from a Web page
        Item item = new Item();
        item.setName(itemName);
        item.setId(count);
        addItem(item);

        //Display information about an item in the console
        System.out.println(count + " = " + itemName); 
        }
    }

    @Override
    public void handleStartTag(HTML.Tag t, MutableAttributeSet a, int pos) {
        if (HTML.Tag.H2 == t) {
            h2Tag = true;
        }
    }

    @Override
    public void handleEndTag(HTML.Tag t, int pos) {
        if (HTML.Tag.H2 == t) {
            h2Tag = false;
        }
    }
}

From another class parser runs as follows:

List<Item> list = Parser.parse();

All is good, but there is a problem. At the end of parsing in the final list "List itemList" contains 980 elements onto, instead of 1000. But in the console there is all of 1000 elements (items). That is, some threads for some reason did not call in the handleText method the addItem method.

I already tried to change the type of itemList to ArrayList, CopyOnWriteArrayList, Vector. Makes the method addItem synchronized, changed its call on the synchronized block. All this only changes the number of elements a little, but the final thousand can not be obtained.

I also tried to parse a smaller number of pages (ten). As the result the list is empty, but in the console all 10.

If I remove multi-threading, then everything works fine, but, of course, slowly. That's not good.

If decrease the number of concurrent threads, the number of items in the list is close to the desired 1000, if increase - a little distanced from 1000. That is, I think, there is a struggle for the ability to record to the list. But then why are synchronization not working?

What's the problem?

Upvotes: 2

Views: 3444

Answers (2)

M.J.
M.J.

Reputation: 16646

There is no problem with the code, it is working as you have coded. the problem is with the last iteration. rest all iterations will work properly, but during the last iteration which is from 980 to 1000, the threads are created, but the main process, does not waits for the other thread to complete, and then return the list. therefore you will be getting some odd number between 980 to 1000, if you are working with 20 threads at a time.

Now you can try adding Thread.wait(50), before returning the list, in that case your main thread will wait, some time, and may be by the time, other threads might finish the processing.

or you can use some syncronization API from java. Instead of Thread.wait(), use CountDownLatch, this will help you to wait for the threads to complete the processing, and then you can create new threads.

Upvotes: 3

Stephan
Stephan

Reputation: 7388

After your parse() call returns, all of your 1000 Threads have been started, but it is not guaranteed that they are finished. In fact, they aren't that's the problem you see. I would heavily recommend not write this by yourself but use the tools provided for this kind of job by the SDK.

The documentation Thread Pools and the ThreadPoolExecutor are e.g. a good starting point. Again, don't implement this yourself if you are not absolutely sure you have too, because writing such multi-threading code is pure pain.

Your code should look something like this: 

ExecutorService executor = Executors.newFixedThreadPool(20);
List<Future<?>> futures = new ArrayList<Future<?>>(1000);
for (int i = 0; i < 1000; i++) { 
   futures.add(executor.submit(new Runnable() {...}));
}
for (Future<?> f : futures) {
   f.get();
}

Upvotes: 5

Related Questions